Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751317AbdGQGm5 (ORCPT ); Mon, 17 Jul 2017 02:42:57 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:54984 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751253AbdGQGmy (ORCPT ); Mon, 17 Jul 2017 02:42:54 -0400 MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Mon, 17 Jul 2017 12:12:52 +0530 From: Abhishek Sahu To: Marek Vasut Cc: dwmw2@infradead.org, computersforpeace@gmail.com, boris.brezillon@free-electrons.com, richard@nod.at, cyrille.pitchen@wedev4u.fr, robh+dt@kernel.org, mark.rutland@arm.com, linux-mtd@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, andy.gross@linaro.org, architt@codeaurora.org, sricharan@codeaurora.org Subject: Re: [PATCH 05/14] qcom: mtd: nand: allocate bam transaction In-Reply-To: <659d69fd-ae7c-b566-ccab-aca2a3efe178@gmail.com> References: <1498720566-20782-1-git-send-email-absahu@codeaurora.org> <1498720566-20782-6-git-send-email-absahu@codeaurora.org> <659d69fd-ae7c-b566-ccab-aca2a3efe178@gmail.com> Message-ID: <1568cf0470fa63676a40318fa45142ad@codeaurora.org> User-Agent: Roundcube Webmail/1.2.5 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7317 Lines: 229 On 2017-06-29 15:20, Marek Vasut wrote: > On 06/29/2017 09:15 AM, Abhishek Sahu wrote: >> The BAM transaction is the core data structure which will be used >> for all the data transfers in QPIC NAND. Since the base layer is >> serializing all the NAND requests so allocating BAM transaction >> before every transfer will be overhead. The memory for it be >> allocated during probe time and before every transfer, it will be >> cleared. The BAM transaction contains the array of command >> elements, command and data scatter gather list and indexes. For >> every transfer, all the resource will be taken from bam >> transaction. >> >> Signed-off-by: Abhishek Sahu >> --- >> drivers/mtd/nand/qcom_nandc.c | 116 >> ++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 116 insertions(+) >> >> diff --git a/drivers/mtd/nand/qcom_nandc.c >> b/drivers/mtd/nand/qcom_nandc.c >> index eb0ec19..f8d0bde 100644 >> --- a/drivers/mtd/nand/qcom_nandc.c >> +++ b/drivers/mtd/nand/qcom_nandc.c >> @@ -22,6 +22,7 @@ >> #include >> #include >> #include >> +#include >> >> /* NANDc reg offsets */ >> #define NAND_FLASH_CMD 0x00 >> @@ -169,6 +170,45 @@ >> #define ECC_BCH_4BIT BIT(2) >> #define ECC_BCH_8BIT BIT(3) >> >> +#define QPIC_PER_CW_MAX_CMD_ELEMENTS (32) >> +#define QPIC_PER_CW_MAX_CMD_SGL (32) >> +#define QPIC_PER_CW_MAX_DATA_SGL (8) >> + >> +/* >> + * This data type corresponds to the BAM transaction which will be >> used >> for all >> + * NAND transfers. >> + * @bam_ce - the array of bam command elements >> + * @cmd_sgl - sgl for nand bam command pipe >> + * @data_sgl - sgl for nand bam consumer/producer pipe >> + * @bam_ce_pos - the index in bam_ce which is available for next sgl >> request >> + * @bam_ce_start - the index in bam_ce which marks the start position >> ce >> + * for current sgl. It will be used for size calculation >> + * for current sgl >> + * @cmd_sgl_pos - current index in command sgl. >> + * @tx_sgl_pos - current index in data sgl for tx. >> + * @rx_sgl_pos - current index in data sgl for rx. >> + */ >> +struct bam_transaction { >> + struct bam_cmd_element *bam_ce; >> + struct scatterlist *cmd_sgl; >> + struct scatterlist *data_sg; >> + u32 bam_ce_pos; >> + u32 bam_ce_start; >> + u32 cmd_sgl_pos; >> + u32 cmd_sgl_start; >> + u32 tx_sgl_pos; >> + u32 tx_sgl_start; >> + u32 rx_sgl_pos; >> + u32 rx_sgl_start; >> +}; >> + >> +/* >> + * This data type corresponds to the nand dma descriptor >> + * @list - list for desc_info >> + * @dir - DMA transfer direction >> + * @sgl - sgl which will be used for single sgl dma descriptor >> + * @dma_desc - low level dma engine descriptor >> + */ >> struct desc_info { >> struct list_head node; >> >> @@ -217,6 +257,7 @@ struct nandc_regs { >> * @aon_clk: another controller clock >> * >> * @chan: dma channel >> + * @bam_txn: contains the bam transaction buffer >> * @cmd_crci: ADM DMA CRCI for command flow control >> * @data_crci: ADM DMA CRCI for data flow control >> * @desc_list: DMA descriptor list (list of desc_infos) >> @@ -237,6 +278,8 @@ struct nandc_regs { >> * initialized via DT match data >> * @dma_bam_enabled: flag to tell whether nand controller is using >> * bam dma >> + * @max_cwperpage: maximum qpic codeword required. calcualted >> + * from all nand device pagesize >> */ >> struct qcom_nand_controller { >> struct nand_hw_control controller; >> @@ -264,12 +307,14 @@ struct qcom_nand_controller { >> }; >> >> struct list_head desc_list; >> + struct bam_transaction *bam_txn; >> >> u8 *data_buffer; >> bool dma_bam_enabled; >> int buf_size; >> int buf_count; >> int buf_start; >> + unsigned int max_cwperpage; >> >> __le32 *reg_read_buf; >> dma_addr_t reg_read_buf_phys; >> @@ -342,6 +387,51 @@ struct qcom_nand_driver_data { >> bool dma_bam_enabled; >> }; >> >> +/* Frees the BAM transaction memory */ >> +static void free_bam_transaction(struct qcom_nand_controller *nandc) >> +{ >> + struct bam_transaction *bam_txn = nandc->bam_txn; >> + >> + devm_kfree(nandc->dev, bam_txn->bam_ce); >> + devm_kfree(nandc->dev, bam_txn->cmd_sgl); >> + devm_kfree(nandc->dev, bam_txn->data_sg); >> + devm_kfree(nandc->dev, bam_txn); >> +} >> + >> +/* 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); > > Can you make these four allocations into a single allocation ? > Sure. I will do the same in v2. >> + 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); >> + if (!nandc->bam_txn) { >> + dev_err(nandc->dev, >> + "failed to allocate bam transaction\n"); >> + return -ENOMEM; >> + } >> + } >> + >> list_for_each_entry_safe(host, tmp, &nandc->host_list, node) { >> ret = qcom_nand_mtd_register(nandc, host, child); >> if (ret) { >> -- Abhishek Sahu