Re: [PATCH 05/14] qcom: mtd: nand: allocate bam transaction

[Abhishek Sahu <absahu@codeaurora.org>]

On 2017-07-03 13:52, Sricharan R wrote:
> Hi Abhishek,
> 
> <..>
> 
>> +/* Allocates and Initializes the BAM transaction */
>> +static struct bam_transaction *
>> +alloc_bam_transaction(struct qcom_nand_controller *nandc, unsigned 
>> int
>> num_cw)
>> +{
>> +	struct bam_transaction *bam_txn;
>> +
>> +	bam_txn = devm_kzalloc(nandc->dev, sizeof(*bam_txn), GFP_KERNEL);
>> +
>> +	if (!bam_txn)
>> +		return NULL;
>> +
>> +	bam_txn->bam_ce =
>> +		devm_kzalloc(nandc->dev, sizeof(*bam_txn->bam_ce) *
>> +			     num_cw * QPIC_PER_CW_MAX_CMD_ELEMENTS, GFP_KERNEL);
>> +	if (!bam_txn->bam_ce)
>> +		return NULL;
>> +
>> +	bam_txn->cmd_sgl =
>> +		devm_kzalloc(nandc->dev, sizeof(*bam_txn->cmd_sgl) * num_cw *
>> +			     QPIC_PER_CW_MAX_CMD_SGL, GFP_KERNEL);
>> +	if (!bam_txn->cmd_sgl)
>> +		return NULL;
>> +
>> +	bam_txn->data_sg =
>> +		devm_kzalloc(nandc->dev, sizeof(*bam_txn->data_sg) *
>> +			     num_cw * QPIC_PER_CW_MAX_DATA_SGL, GFP_KERNEL);
>> +	if (!bam_txn->data_sg)
>> +		return NULL;
>> +
>> +	nandc->max_cwperpage = num_cw;
>> +
>> +	return bam_txn;
>> +}
>> +
>>  static inline struct qcom_nand_host *to_qcom_nand_host(struct 
>> nand_chip
>> *chip)
>>  {
>>  	return container_of(chip, struct qcom_nand_host, chip);
>> @@ -1868,6 +1958,8 @@ static int qcom_nand_host_setup(struct
>> qcom_nand_host *host)
>>  	mtd_set_ooblayout(mtd, &qcom_nand_ooblayout_ops);
>> 
>>  	cwperpage = mtd->writesize / ecc->size;
>> +	nandc->max_cwperpage = max_t(unsigned int, nandc->max_cwperpage,
>> +					cwperpage);
>> 
>>  	/*
>>  	 * DATA_UD_BYTES varies based on whether the read/write command 
>> protects
>> @@ -2010,6 +2102,19 @@ static int qcom_nandc_alloc(struct
>> qcom_nand_controller *nandc)
>>  			dev_err(nandc->dev, "failed to request cmd channel\n");
>>  			return -ENODEV;
>>  		}
>> +
>> +		/*
>> +		 * Initially allocate BAM transaction to read ONFI param page.
>> +		 * After detecting all the devices, this BAM transaction will
>> +		 * be freed and the next BAM tranasction will be allocated with
>> +		 * maximum codeword size
>> +		 */
>> +		nandc->bam_txn = alloc_bam_transaction(nandc, 1);
>> +		if (!nandc->bam_txn) {
>> +			dev_err(nandc->dev,
>> +				"failed to allocate bam transaction\n");
>> +			return -ENOMEM;
>> +		}
>>  	}
>> 
>>  	INIT_LIST_HEAD(&nandc->desc_list);
>> @@ -2153,6 +2258,17 @@ static int qcom_probe_nand_devices(struct
>> qcom_nand_controller *nandc)
>>  	if (list_empty(&nandc->host_list))
>>  		return -ENODEV;
>> 
>> +	if (nandc->dma_bam_enabled) {
>> +		free_bam_transaction(nandc);
>> +		nandc->bam_txn = alloc_bam_transaction(nandc,
>> +						       nandc->max_cwperpage);
> 
>  Somehow, looks like something is missing because, nandc->max_cwperpage
> passed from
>  here is used in alloc_bam_transaction, but it is assigned in the same
> function ?
> 

  Yes. This assignment is not required. I will fix this in v2.

> Regards,
>  Sricharan

-- 
Abhishek Sahu