| Message ID | 1498720566-20782-3-git-send-email-absahu@codeaurora.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Boris Brezillon |
| Headers | show |
On 06/29/2017 09:15 AM, Abhishek Sahu wrote: > 1. The QPIC NAND uses 3 BAM channels: command, data tx and > data rx while EBI2 NAND uses only single ADM channel. > > 2. The EBI2 NAND uses normal register read buffer since this > buffer will be remapped with dma_map_sg. The QPIC NAND will give > register read buffer in command descriptor and the command > descriptor will be mapped with dma_map_sg so the register buffer > should be DMA coherent. > > Signed-off-by: Abhishek Sahu <absahu@codeaurora.org> The patch does two things, so make two patches. Also split the DT changes into separate patch ... > --- > .../devicetree/bindings/mtd/qcom_nandc.txt | 25 +++-- > drivers/mtd/nand/qcom_nandc.c | 106 ++++++++++++++++----- > 2 files changed, 99 insertions(+), 32 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt > index 5d0f7ae..87b9a56 100644 > --- a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt > +++ b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt > @@ -9,15 +9,17 @@ Required properties: > - clock-names: must contain "core" for the core clock and "aon" for the > always on clock > - dmas: DMA specifier, consisting of a phandle to the ADM DMA > - controller node and the channel number to be used for > - NAND. Refer to dma.txt and qcom_adm.txt for more details > -- dma-names: must be "rxtx" > -- qcom,cmd-crci: must contain the ADM command type CRCI block instance > - number specified for the NAND controller on the given > - platform > -- qcom,data-crci: must contain the ADM data type CRCI block instance > - number specified for the NAND controller on the given > - platform > + or BAM DMA controller node and the channel number to > + be used for NAND. Refer to dma.txt, qcom_adm.txt(ADM) > + and qcom/bam_dma.txt(BAM) for more details > +- dma-names: "rxtx" - ADM > + "tx", "rx", "cmd" - BAM > +- qcom,cmd-crci: Only required for ADM DMA. must contain the ADM command > + type CRCI block instance number specified for the NAND > + controller on the given platform. > +- qcom,data-crci: Only required for ADM DMA. must contain the ADM data > + type CRCI block instance number specified for the NAND > + controller on the given platform. > - #address-cells: <1> - subnodes give the chip-select number > - #size-cells: <0> > > @@ -95,6 +97,11 @@ nand@79b0000 { > <&gcc GCC_QPIC_AHB_CLK>; > clock-names = "core", "aon"; > > + dmas = <&qpicbam 0>, > + <&qpicbam 1>, > + <&qpicbam 2>; > + dma-names = "tx", "rx", "cmd"; > + > #address-cells = <1>; > #size-cells = <0>; > > diff --git a/drivers/mtd/nand/qcom_nandc.c b/drivers/mtd/nand/qcom_nandc.c > index f55f728..520add9 100644 > --- a/drivers/mtd/nand/qcom_nandc.c > +++ b/drivers/mtd/nand/qcom_nandc.c > @@ -226,6 +226,7 @@ struct nandc_regs { > * by upper layers directly > * @buf_size/count/start: markers for chip->read_buf/write_buf functions > * @reg_read_buf: local buffer for reading back registers via DMA > + * @reg_read_buf_phys: contains dma address for register read buffer > * @reg_read_pos: marker for data read in reg_read_buf > * > * @regs: a contiguous chunk of memory for DMA register > @@ -249,9 +250,19 @@ struct qcom_nand_controller { > struct clk *core_clk; > struct clk *aon_clk; > > - struct dma_chan *chan; > - unsigned int cmd_crci; > - unsigned int data_crci; > + union { > + struct { > + struct dma_chan *tx_chan; > + struct dma_chan *rx_chan; > + struct dma_chan *cmd_chan; > + }; > + struct { > + struct dma_chan *chan; > + unsigned int cmd_crci; > + unsigned int data_crci; > + }; > + }; > + > struct list_head desc_list; > > u8 *data_buffer; > @@ -261,6 +272,7 @@ struct qcom_nand_controller { > int buf_start; > > __le32 *reg_read_buf; > + dma_addr_t reg_read_buf_phys; > int reg_read_pos; > > struct nandc_regs *regs; > @@ -1956,16 +1968,48 @@ static int qcom_nandc_alloc(struct qcom_nand_controller *nandc) > if (!nandc->regs) > return -ENOMEM; > > - nandc->reg_read_buf = devm_kzalloc(nandc->dev, > - MAX_REG_RD * sizeof(*nandc->reg_read_buf), > - GFP_KERNEL); > - if (!nandc->reg_read_buf) > - return -ENOMEM; > > - nandc->chan = dma_request_slave_channel(nandc->dev, "rxtx"); > - if (!nandc->chan) { > - dev_err(nandc->dev, "failed to request slave channel\n"); > - return -ENODEV; > + if (!nandc->dma_bam_enabled) { > + nandc->reg_read_buf = > + devm_kzalloc(nandc->dev, MAX_REG_RD * > + sizeof(*nandc->reg_read_buf), GFP_KERNEL); > + > + if (!nandc->reg_read_buf) > + return -ENOMEM; > + > + nandc->chan = dma_request_slave_channel(nandc->dev, "rxtx"); > + if (!nandc->chan) { > + dev_err(nandc->dev, > + "failed to request slave channel\n"); > + return -ENODEV; > + } > + } else { > + nandc->reg_read_buf = > + dmam_alloc_coherent(nandc->dev, MAX_REG_RD * > + sizeof(*nandc->reg_read_buf), > + &nandc->reg_read_buf_phys, > + GFP_KERNEL); > + > + if (!nandc->reg_read_buf) > + return -ENOMEM; > + > + nandc->tx_chan = dma_request_slave_channel(nandc->dev, "tx"); > + if (!nandc->tx_chan) { > + dev_err(nandc->dev, "failed to request tx channel\n"); > + return -ENODEV; > + } > + > + nandc->rx_chan = dma_request_slave_channel(nandc->dev, "rx"); > + if (!nandc->rx_chan) { > + dev_err(nandc->dev, "failed to request rx channel\n"); > + return -ENODEV; > + } > + > + nandc->cmd_chan = dma_request_slave_channel(nandc->dev, "cmd"); > + if (!nandc->cmd_chan) { > + dev_err(nandc->dev, "failed to request cmd channel\n"); > + return -ENODEV; > + } > } > > INIT_LIST_HEAD(&nandc->desc_list); > @@ -1978,7 +2022,19 @@ static int qcom_nandc_alloc(struct qcom_nand_controller *nandc) > > static void qcom_nandc_unalloc(struct qcom_nand_controller *nandc) > { > - dma_release_channel(nandc->chan); > + if (nandc->dma_bam_enabled) { > + if (nandc->tx_chan) > + dma_release_channel(nandc->tx_chan); > + > + if (nandc->rx_chan) > + dma_release_channel(nandc->rx_chan); > + > + if (nandc->cmd_chan) > + dma_release_channel(nandc->cmd_chan); > + } else { > + if (nandc->chan) > + dma_release_channel(nandc->chan); > + } > } > > /* one time setup of a few nand controller registers */ > @@ -2063,16 +2119,20 @@ static int qcom_nandc_parse_dt(struct platform_device *pdev) > struct device_node *np = nandc->dev->of_node; > int ret; > > - ret = of_property_read_u32(np, "qcom,cmd-crci", &nandc->cmd_crci); > - if (ret) { > - dev_err(nandc->dev, "command CRCI unspecified\n"); > - return ret; > - } > + if (!nandc->dma_bam_enabled) { > + ret = of_property_read_u32(np, "qcom,cmd-crci", > + &nandc->cmd_crci); > + if (ret) { > + dev_err(nandc->dev, "command CRCI unspecified\n"); > + return ret; > + } > > - ret = of_property_read_u32(np, "qcom,data-crci", &nandc->data_crci); > - if (ret) { > - dev_err(nandc->dev, "data CRCI unspecified\n"); > - return ret; > + ret = of_property_read_u32(np, "qcom,data-crci", > + &nandc->data_crci); > + if (ret) { > + dev_err(nandc->dev, "data CRCI unspecified\n"); > + return ret; > + } > } > > return 0; > @@ -2128,7 +2188,7 @@ static int qcom_nandc_probe(struct platform_device *pdev) > > ret = qcom_nandc_alloc(nandc); > if (ret) > - return ret; > + goto err_core_clk; > > ret = clk_prepare_enable(nandc->core_clk); > if (ret) > Can you please fix your mailer to stop adding "QUALCOMM INDIA, on behalf of Qualcomm Innovation Center"... stuff at the bottom of the patches ?
On 06/29/2017 12:45 PM, Abhishek Sahu wrote: > 1. The QPIC NAND uses 3 BAM channels: command, data tx and > data rx while EBI2 NAND uses only single ADM channel. > > 2. The EBI2 NAND uses normal register read buffer since this > buffer will be remapped with dma_map_sg. The QPIC NAND will give > register read buffer in command descriptor and the command > descriptor will be mapped with dma_map_sg so the register buffer > should be DMA coherent. It isn't entirely clear from this commit message why we require reg_read_buf to be DMA coherent for QPIC NAND. Could you please explain this better? Besides Marek's comment to splitting the patch, it looks okay to me. Thanks, Archit > > Signed-off-by: Abhishek Sahu <absahu@codeaurora.org> > --- > .../devicetree/bindings/mtd/qcom_nandc.txt | 25 +++-- > drivers/mtd/nand/qcom_nandc.c | 106 ++++++++++++++++----- > 2 files changed, 99 insertions(+), 32 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt > index 5d0f7ae..87b9a56 100644 > --- a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt > +++ b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt > @@ -9,15 +9,17 @@ Required properties: > - clock-names: must contain "core" for the core clock and "aon" for the > always on clock > - dmas: DMA specifier, consisting of a phandle to the ADM DMA > - controller node and the channel number to be used for > - NAND. Refer to dma.txt and qcom_adm.txt for more details > -- dma-names: must be "rxtx" > -- qcom,cmd-crci: must contain the ADM command type CRCI block instance > - number specified for the NAND controller on the given > - platform > -- qcom,data-crci: must contain the ADM data type CRCI block instance > - number specified for the NAND controller on the given > - platform > + or BAM DMA controller node and the channel number to > + be used for NAND. Refer to dma.txt, qcom_adm.txt(ADM) > + and qcom/bam_dma.txt(BAM) for more details > +- dma-names: "rxtx" - ADM > + "tx", "rx", "cmd" - BAM > +- qcom,cmd-crci: Only required for ADM DMA. must contain the ADM command > + type CRCI block instance number specified for the NAND > + controller on the given platform. > +- qcom,data-crci: Only required for ADM DMA. must contain the ADM data > + type CRCI block instance number specified for the NAND > + controller on the given platform. > - #address-cells: <1> - subnodes give the chip-select number > - #size-cells: <0> > > @@ -95,6 +97,11 @@ nand@79b0000 { > <&gcc GCC_QPIC_AHB_CLK>; > clock-names = "core", "aon"; > > + dmas = <&qpicbam 0>, > + <&qpicbam 1>, > + <&qpicbam 2>; > + dma-names = "tx", "rx", "cmd"; > + > #address-cells = <1>; > #size-cells = <0>; > > diff --git a/drivers/mtd/nand/qcom_nandc.c b/drivers/mtd/nand/qcom_nandc.c > index f55f728..520add9 100644 > --- a/drivers/mtd/nand/qcom_nandc.c > +++ b/drivers/mtd/nand/qcom_nandc.c > @@ -226,6 +226,7 @@ struct nandc_regs { > * by upper layers directly > * @buf_size/count/start: markers for chip->read_buf/write_buf functions > * @reg_read_buf: local buffer for reading back registers via DMA > + * @reg_read_buf_phys: contains dma address for register read buffer > * @reg_read_pos: marker for data read in reg_read_buf > * > * @regs: a contiguous chunk of memory for DMA register > @@ -249,9 +250,19 @@ struct qcom_nand_controller { > struct clk *core_clk; > struct clk *aon_clk; > > - struct dma_chan *chan; > - unsigned int cmd_crci; > - unsigned int data_crci; > + union { > + struct { > + struct dma_chan *tx_chan; > + struct dma_chan *rx_chan; > + struct dma_chan *cmd_chan; > + }; > + struct { > + struct dma_chan *chan; > + unsigned int cmd_crci; > + unsigned int data_crci; > + }; > + }; > + > struct list_head desc_list; > > u8 *data_buffer; > @@ -261,6 +272,7 @@ struct qcom_nand_controller { > int buf_start; > > __le32 *reg_read_buf; > + dma_addr_t reg_read_buf_phys; > int reg_read_pos; > > struct nandc_regs *regs; > @@ -1956,16 +1968,48 @@ static int qcom_nandc_alloc(struct qcom_nand_controller *nandc) > if (!nandc->regs) > return -ENOMEM; > > - nandc->reg_read_buf = devm_kzalloc(nandc->dev, > - MAX_REG_RD * sizeof(*nandc->reg_read_buf), > - GFP_KERNEL); > - if (!nandc->reg_read_buf) > - return -ENOMEM; > > - nandc->chan = dma_request_slave_channel(nandc->dev, "rxtx"); > - if (!nandc->chan) { > - dev_err(nandc->dev, "failed to request slave channel\n"); > - return -ENODEV; > + if (!nandc->dma_bam_enabled) { > + nandc->reg_read_buf = > + devm_kzalloc(nandc->dev, MAX_REG_RD * > + sizeof(*nandc->reg_read_buf), GFP_KERNEL); > + > + if (!nandc->reg_read_buf) > + return -ENOMEM; > + > + nandc->chan = dma_request_slave_channel(nandc->dev, "rxtx"); > + if (!nandc->chan) { > + dev_err(nandc->dev, > + "failed to request slave channel\n"); > + return -ENODEV; > + } > + } else { > + nandc->reg_read_buf = > + dmam_alloc_coherent(nandc->dev, MAX_REG_RD * > + sizeof(*nandc->reg_read_buf), > + &nandc->reg_read_buf_phys, > + GFP_KERNEL); > + > + if (!nandc->reg_read_buf) > + return -ENOMEM; > + > + nandc->tx_chan = dma_request_slave_channel(nandc->dev, "tx"); > + if (!nandc->tx_chan) { > + dev_err(nandc->dev, "failed to request tx channel\n"); > + return -ENODEV; > + } > + > + nandc->rx_chan = dma_request_slave_channel(nandc->dev, "rx"); > + if (!nandc->rx_chan) { > + dev_err(nandc->dev, "failed to request rx channel\n"); > + return -ENODEV; > + } > + > + nandc->cmd_chan = dma_request_slave_channel(nandc->dev, "cmd"); > + if (!nandc->cmd_chan) { > + dev_err(nandc->dev, "failed to request cmd channel\n"); > + return -ENODEV; > + } > } > > INIT_LIST_HEAD(&nandc->desc_list); > @@ -1978,7 +2022,19 @@ static int qcom_nandc_alloc(struct qcom_nand_controller *nandc) > > static void qcom_nandc_unalloc(struct qcom_nand_controller *nandc) > { > - dma_release_channel(nandc->chan); > + if (nandc->dma_bam_enabled) { > + if (nandc->tx_chan) > + dma_release_channel(nandc->tx_chan); > + > + if (nandc->rx_chan) > + dma_release_channel(nandc->rx_chan); > + > + if (nandc->cmd_chan) > + dma_release_channel(nandc->cmd_chan); > + } else { > + if (nandc->chan) > + dma_release_channel(nandc->chan); > + } > } > > /* one time setup of a few nand controller registers */ > @@ -2063,16 +2119,20 @@ static int qcom_nandc_parse_dt(struct platform_device *pdev) > struct device_node *np = nandc->dev->of_node; > int ret; > > - ret = of_property_read_u32(np, "qcom,cmd-crci", &nandc->cmd_crci); > - if (ret) { > - dev_err(nandc->dev, "command CRCI unspecified\n"); > - return ret; > - } > + if (!nandc->dma_bam_enabled) { > + ret = of_property_read_u32(np, "qcom,cmd-crci", > + &nandc->cmd_crci); > + if (ret) { > + dev_err(nandc->dev, "command CRCI unspecified\n"); > + return ret; > + } > > - ret = of_property_read_u32(np, "qcom,data-crci", &nandc->data_crci); > - if (ret) { > - dev_err(nandc->dev, "data CRCI unspecified\n"); > - return ret; > + ret = of_property_read_u32(np, "qcom,data-crci", > + &nandc->data_crci); > + if (ret) { > + dev_err(nandc->dev, "data CRCI unspecified\n"); > + return ret; > + } > } > > return 0; > @@ -2128,7 +2188,7 @@ static int qcom_nandc_probe(struct platform_device *pdev) > > ret = qcom_nandc_alloc(nandc); > if (ret) > - return ret; > + goto err_core_clk; > > ret = clk_prepare_enable(nandc->core_clk); > if (ret) >
Hi Abhishek, On 6/29/2017 12:45 PM, Abhishek Sahu wrote: > 1. The QPIC NAND uses 3 BAM channels: command, data tx and > data rx while EBI2 NAND uses only single ADM channel. > > 2. The EBI2 NAND uses normal register read buffer since this > buffer will be remapped with dma_map_sg. The QPIC NAND will give > register read buffer in command descriptor and the command > descriptor will be mapped with dma_map_sg so the register buffer > should be DMA coherent. > > Signed-off-by: Abhishek Sahu <absahu@codeaurora.org> > --- > .../devicetree/bindings/mtd/qcom_nandc.txt | 25 +++-- > drivers/mtd/nand/qcom_nandc.c | 106 ++++++++++++++++----- > 2 files changed, 99 insertions(+), 32 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt > index 5d0f7ae..87b9a56 100644 > --- a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt > +++ b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt > @@ -9,15 +9,17 @@ Required properties: > - clock-names: must contain "core" for the core clock and "aon" for the > always on clock > - dmas: DMA specifier, consisting of a phandle to the ADM DMA > - controller node and the channel number to be used for > - NAND. Refer to dma.txt and qcom_adm.txt for more details > -- dma-names: must be "rxtx" > -- qcom,cmd-crci: must contain the ADM command type CRCI block instance > - number specified for the NAND controller on the given > - platform > -- qcom,data-crci: must contain the ADM data type CRCI block instance > - number specified for the NAND controller on the given > - platform > + or BAM DMA controller node and the channel number to > + be used for NAND. Refer to dma.txt, qcom_adm.txt(ADM) > + and qcom/bam_dma.txt(BAM) for more details > +- dma-names: "rxtx" - ADM > + "tx", "rx", "cmd" - BAM > +- qcom,cmd-crci: Only required for ADM DMA. must contain the ADM command > + type CRCI block instance number specified for the NAND > + controller on the given platform. > +- qcom,data-crci: Only required for ADM DMA. must contain the ADM data > + type CRCI block instance number specified for the NAND > + controller on the given platform. May be better to put the parameter list that are specific to ADMA and BAM under seperate groups and those common for both in one group, instead of mentioning at each of the property level. > - #address-cells: <1> - subnodes give the chip-select number > - #size-cells: <0> > > @@ -95,6 +97,11 @@ nand@79b0000 { > <&gcc GCC_QPIC_AHB_CLK>; > clock-names = "core", "aon"; > > + dmas = <&qpicbam 0>, > + <&qpicbam 1>, > + <&qpicbam 2>; > + dma-names = "tx", "rx", "cmd"; > + > #address-cells = <1>; > #size-cells = <0>; > > diff --git a/drivers/mtd/nand/qcom_nandc.c b/drivers/mtd/nand/qcom_nandc.c > index f55f728..520add9 100644 > --- a/drivers/mtd/nand/qcom_nandc.c > +++ b/drivers/mtd/nand/qcom_nandc.c > @@ -226,6 +226,7 @@ struct nandc_regs { > * by upper layers directly > * @buf_size/count/start: markers for chip->read_buf/write_buf functions > * @reg_read_buf: local buffer for reading back registers via DMA > + * @reg_read_buf_phys: contains dma address for register read buffer > * @reg_read_pos: marker for data read in reg_read_buf > * > * @regs: a contiguous chunk of memory for DMA register > @@ -249,9 +250,19 @@ struct qcom_nand_controller { > struct clk *core_clk; > struct clk *aon_clk; > > - struct dma_chan *chan; > - unsigned int cmd_crci; > - unsigned int data_crci; > + union { > + struct { > + struct dma_chan *tx_chan; > + struct dma_chan *rx_chan; > + struct dma_chan *cmd_chan; > + }; > + struct { > + struct dma_chan *chan; > + unsigned int cmd_crci; > + unsigned int data_crci; > + }; > + }; > + > struct list_head desc_list; > > u8 *data_buffer; > @@ -261,6 +272,7 @@ struct qcom_nand_controller { > int buf_start; > > __le32 *reg_read_buf; > + dma_addr_t reg_read_buf_phys; > int reg_read_pos; > > struct nandc_regs *regs; > @@ -1956,16 +1968,48 @@ static int qcom_nandc_alloc(struct qcom_nand_controller *nandc) > if (!nandc->regs) > return -ENOMEM; > > - nandc->reg_read_buf = devm_kzalloc(nandc->dev, > - MAX_REG_RD * sizeof(*nandc->reg_read_buf), > - GFP_KERNEL); > - if (!nandc->reg_read_buf) > - return -ENOMEM; > > - nandc->chan = dma_request_slave_channel(nandc->dev, "rxtx"); > - if (!nandc->chan) { > - dev_err(nandc->dev, "failed to request slave channel\n"); > - return -ENODEV; > + if (!nandc->dma_bam_enabled) { Better to swap if, else part to avoid ! > + nandc->reg_read_buf = > + devm_kzalloc(nandc->dev, MAX_REG_RD * > + sizeof(*nandc->reg_read_buf), GFP_KERNEL); > + > + if (!nandc->reg_read_buf) > + return -ENOMEM; > + > + nandc->chan = dma_request_slave_channel(nandc->dev, "rxtx"); > + if (!nandc->chan) { > + dev_err(nandc->dev, > + "failed to request slave channel\n"); > + return -ENODEV; > + } > + } else { > + nandc->reg_read_buf = > + dmam_alloc_coherent(nandc->dev, MAX_REG_RD * > + sizeof(*nandc->reg_read_buf), > + &nandc->reg_read_buf_phys, > + GFP_KERNEL); > + > + if (!nandc->reg_read_buf) > + return -ENOMEM; > + > + nandc->tx_chan = dma_request_slave_channel(nandc->dev, "tx"); > + if (!nandc->tx_chan) { > + dev_err(nandc->dev, "failed to request tx channel\n"); > + return -ENODEV; > + } > + > + nandc->rx_chan = dma_request_slave_channel(nandc->dev, "rx"); > + if (!nandc->rx_chan) { > + dev_err(nandc->dev, "failed to request rx channel\n"); > + return -ENODEV; > + } > + > + nandc->cmd_chan = dma_request_slave_channel(nandc->dev, "cmd"); > + if (!nandc->cmd_chan) { > + dev_err(nandc->dev, "failed to request cmd channel\n"); > + return -ENODEV; > + } > } > > INIT_LIST_HEAD(&nandc->desc_list); > @@ -1978,7 +2022,19 @@ static int qcom_nandc_alloc(struct qcom_nand_controller *nandc) > > static void qcom_nandc_unalloc(struct qcom_nand_controller *nandc) > { > - dma_release_channel(nandc->chan); > + if (nandc->dma_bam_enabled) { > + if (nandc->tx_chan) > + dma_release_channel(nandc->tx_chan); > + > + if (nandc->rx_chan) > + dma_release_channel(nandc->rx_chan); > + > + if (nandc->cmd_chan) > + dma_release_channel(nandc->cmd_chan); > + } else { > + if (nandc->chan) > + dma_release_channel(nandc->chan); > + } > } > > /* one time setup of a few nand controller registers */ > @@ -2063,16 +2119,20 @@ static int qcom_nandc_parse_dt(struct platform_device *pdev) > struct device_node *np = nandc->dev->of_node; > int ret; > > - ret = of_property_read_u32(np, "qcom,cmd-crci", &nandc->cmd_crci); > - if (ret) { > - dev_err(nandc->dev, "command CRCI unspecified\n"); > - return ret; > - } > + if (!nandc->dma_bam_enabled) { instead, can we simply do if (is_bam) return; here ? Regards, Sricharan
Hi Abhishek, On 6/29/2017 12:45 PM, Abhishek Sahu wrote: > 1. The QPIC NAND uses 3 BAM channels: command, data tx and > data rx while EBI2 NAND uses only single ADM channel. > > 2. The EBI2 NAND uses normal register read buffer since this > buffer will be remapped with dma_map_sg. The QPIC NAND will give > register read buffer in command descriptor and the command > descriptor will be mapped with dma_map_sg so the register buffer > should be DMA coherent. > > Signed-off-by: Abhishek Sahu <absahu@codeaurora.org> > --- > .../devicetree/bindings/mtd/qcom_nandc.txt | 25 +++-- > drivers/mtd/nand/qcom_nandc.c | 106 ++++++++++++++++----- > 2 files changed, 99 insertions(+), 32 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt > index 5d0f7ae..87b9a56 100644 > --- a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt > +++ b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt > @@ -9,15 +9,17 @@ Required properties: > - clock-names: must contain "core" for the core clock and "aon" for the > always on clock > - dmas: DMA specifier, consisting of a phandle to the ADM DMA > - controller node and the channel number to be used for > - NAND. Refer to dma.txt and qcom_adm.txt for more details > -- dma-names: must be "rxtx" > -- qcom,cmd-crci: must contain the ADM command type CRCI block instance > - number specified for the NAND controller on the given > - platform > -- qcom,data-crci: must contain the ADM data type CRCI block instance > - number specified for the NAND controller on the given > - platform > + or BAM DMA controller node and the channel number to > + be used for NAND. Refer to dma.txt, qcom_adm.txt(ADM) > + and qcom/bam_dma.txt(BAM) for more details > +- dma-names: "rxtx" - ADM > + "tx", "rx", "cmd" - BAM > +- qcom,cmd-crci: Only required for ADM DMA. must contain the ADM command > + type CRCI block instance number specified for the NAND > + controller on the given platform. > +- qcom,data-crci: Only required for ADM DMA. must contain the ADM data > + type CRCI block instance number specified for the NAND > + controller on the given platform. > - #address-cells: <1> - subnodes give the chip-select number > - #size-cells: <0> > > @@ -95,6 +97,11 @@ nand@79b0000 { > <&gcc GCC_QPIC_AHB_CLK>; > clock-names = "core", "aon"; > > + dmas = <&qpicbam 0>, > + <&qpicbam 1>, > + <&qpicbam 2>; > + dma-names = "tx", "rx", "cmd"; > + > #address-cells = <1>; > #size-cells = <0>; > > diff --git a/drivers/mtd/nand/qcom_nandc.c b/drivers/mtd/nand/qcom_nandc.c > index f55f728..520add9 100644 > --- a/drivers/mtd/nand/qcom_nandc.c > +++ b/drivers/mtd/nand/qcom_nandc.c > @@ -226,6 +226,7 @@ struct nandc_regs { > * by upper layers directly > * @buf_size/count/start: markers for chip->read_buf/write_buf functions > * @reg_read_buf: local buffer for reading back registers via DMA > + * @reg_read_buf_phys: contains dma address for register read buffer > * @reg_read_pos: marker for data read in reg_read_buf > * > * @regs: a contiguous chunk of memory for DMA register > @@ -249,9 +250,19 @@ struct qcom_nand_controller { > struct clk *core_clk; > struct clk *aon_clk; > > - struct dma_chan *chan; > - unsigned int cmd_crci; > - unsigned int data_crci; > + union { > + struct { > + struct dma_chan *tx_chan; > + struct dma_chan *rx_chan; > + struct dma_chan *cmd_chan; > + }; > + struct { > + struct dma_chan *chan; > + unsigned int cmd_crci; > + unsigned int data_crci; > + }; > + }; > + > struct list_head desc_list; > > u8 *data_buffer; > @@ -261,6 +272,7 @@ struct qcom_nand_controller { > int buf_start; > > __le32 *reg_read_buf; > + dma_addr_t reg_read_buf_phys; > int reg_read_pos; > > struct nandc_regs *regs; > @@ -1956,16 +1968,48 @@ static int qcom_nandc_alloc(struct qcom_nand_controller *nandc) > if (!nandc->regs) > return -ENOMEM; > > - nandc->reg_read_buf = devm_kzalloc(nandc->dev, > - MAX_REG_RD * sizeof(*nandc->reg_read_buf), > - GFP_KERNEL); > - if (!nandc->reg_read_buf) > - return -ENOMEM; > > - nandc->chan = dma_request_slave_channel(nandc->dev, "rxtx"); > - if (!nandc->chan) { > - dev_err(nandc->dev, "failed to request slave channel\n"); > - return -ENODEV; > + if (!nandc->dma_bam_enabled) { > + nandc->reg_read_buf = > + devm_kzalloc(nandc->dev, MAX_REG_RD * > + sizeof(*nandc->reg_read_buf), GFP_KERNEL); > + > + if (!nandc->reg_read_buf) > + return -ENOMEM; > + > + nandc->chan = dma_request_slave_channel(nandc->dev, "rxtx"); > + if (!nandc->chan) { > + dev_err(nandc->dev, > + "failed to request slave channel\n"); > + return -ENODEV; > + } > + } else { > + nandc->reg_read_buf = > + dmam_alloc_coherent(nandc->dev, MAX_REG_RD * > + sizeof(*nandc->reg_read_buf), > + &nandc->reg_read_buf_phys, > + GFP_KERNEL); > + I think as Archit has commented, do not see a reason for this to be alloc_coherent change here. Regards, Sricharan
On 2017-07-03 10:47, Archit Taneja wrote: > On 06/29/2017 12:45 PM, Abhishek Sahu wrote: >> 1. The QPIC NAND uses 3 BAM channels: command, data tx and >> data rx while EBI2 NAND uses only single ADM channel. >> >> 2. The EBI2 NAND uses normal register read buffer since this >> buffer will be remapped with dma_map_sg. The QPIC NAND will give >> register read buffer in command descriptor and the command >> descriptor will be mapped with dma_map_sg so the register buffer >> should be DMA coherent. > > It isn't entirely clear from this commit message why we require > reg_read_buf to be DMA coherent for QPIC NAND. Could you please explain > this > better? I have used DMA coherent since we need to pass this memory in command descriptor where BAM will fill the register contents. Now for v2, I have planned to use streaming DMA API's and its working fine. > > Besides Marek's comment to splitting the patch, it looks okay to me. > > Thanks, > Archit > >> >> Signed-off-by: Abhishek Sahu <absahu@codeaurora.org> >> --- >> .../devicetree/bindings/mtd/qcom_nandc.txt | 25 +++-- >> drivers/mtd/nand/qcom_nandc.c | 106 >> ++++++++++++++++----- >> 2 files changed, 99 insertions(+), 32 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt >> b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt >> index 5d0f7ae..87b9a56 100644 >> --- a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt >> +++ b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt >> @@ -9,15 +9,17 @@ Required properties: >> - clock-names: must contain "core" for the core clock and "aon" for >> the >> always on clock >> - dmas: DMA specifier, consisting of a phandle to the ADM DMA >> - controller node and the channel number to be used for >> - NAND. Refer to dma.txt and qcom_adm.txt for more details >> -- dma-names: must be "rxtx" >> -- qcom,cmd-crci: must contain the ADM command type CRCI block >> instance >> - number specified for the NAND controller on the given >> - platform >> -- qcom,data-crci: must contain the ADM data type CRCI block instance >> - number specified for the NAND controller on the given >> - platform >> + or BAM DMA controller node and the channel number to >> + be used for NAND. Refer to dma.txt, qcom_adm.txt(ADM) >> + and qcom/bam_dma.txt(BAM) for more details >> +- dma-names: "rxtx" - ADM >> + "tx", "rx", "cmd" - BAM >> +- qcom,cmd-crci: Only required for ADM DMA. must contain the ADM >> command >> + type CRCI block instance number specified for the NAND >> + controller on the given platform. >> +- qcom,data-crci: Only required for ADM DMA. must contain the ADM >> data >> + type CRCI block instance number specified for the NAND >> + controller on the given platform. >> - #address-cells: <1> - subnodes give the chip-select number >> - #size-cells: <0> >> >> @@ -95,6 +97,11 @@ nand@79b0000 { >> <&gcc GCC_QPIC_AHB_CLK>; >> clock-names = "core", "aon"; >> >> + dmas = <&qpicbam 0>, >> + <&qpicbam 1>, >> + <&qpicbam 2>; >> + dma-names = "tx", "rx", "cmd"; >> + >> #address-cells = <1>; >> #size-cells = <0>; >> >> diff --git a/drivers/mtd/nand/qcom_nandc.c >> b/drivers/mtd/nand/qcom_nandc.c >> index f55f728..520add9 100644 >> --- a/drivers/mtd/nand/qcom_nandc.c >> +++ b/drivers/mtd/nand/qcom_nandc.c >> @@ -226,6 +226,7 @@ struct nandc_regs { >> * by upper layers directly >> * @buf_size/count/start: markers for chip->read_buf/write_buf >> functions >> * @reg_read_buf: local buffer for reading back registers via DMA >> + * @reg_read_buf_phys: contains dma address for register read buffer >> * @reg_read_pos: marker for data read in reg_read_buf >> * >> * @regs: a contiguous chunk of memory for DMA register >> @@ -249,9 +250,19 @@ struct qcom_nand_controller { >> struct clk *core_clk; >> struct clk *aon_clk; >> >> - struct dma_chan *chan; >> - unsigned int cmd_crci; >> - unsigned int data_crci; >> + union { >> + struct { >> + struct dma_chan *tx_chan; >> + struct dma_chan *rx_chan; >> + struct dma_chan *cmd_chan; >> + }; >> + struct { >> + struct dma_chan *chan; >> + unsigned int cmd_crci; >> + unsigned int data_crci; >> + }; >> + }; >> + >> struct list_head desc_list; >> >> u8 *data_buffer; >> @@ -261,6 +272,7 @@ struct qcom_nand_controller { >> int buf_start; >> >> __le32 *reg_read_buf; >> + dma_addr_t reg_read_buf_phys; >> int reg_read_pos; >> >> struct nandc_regs *regs; >> @@ -1956,16 +1968,48 @@ static int qcom_nandc_alloc(struct >> qcom_nand_controller *nandc) >> if (!nandc->regs) >> return -ENOMEM; >> >> - nandc->reg_read_buf = devm_kzalloc(nandc->dev, >> - MAX_REG_RD * sizeof(*nandc->reg_read_buf), >> - GFP_KERNEL); >> - if (!nandc->reg_read_buf) >> - return -ENOMEM; >> >> - nandc->chan = dma_request_slave_channel(nandc->dev, "rxtx"); >> - if (!nandc->chan) { >> - dev_err(nandc->dev, "failed to request slave channel\n"); >> - return -ENODEV; >> + if (!nandc->dma_bam_enabled) { >> + nandc->reg_read_buf = >> + devm_kzalloc(nandc->dev, MAX_REG_RD * >> + sizeof(*nandc->reg_read_buf), GFP_KERNEL); >> + >> + if (!nandc->reg_read_buf) >> + return -ENOMEM; >> + >> + nandc->chan = dma_request_slave_channel(nandc->dev, "rxtx"); >> + if (!nandc->chan) { >> + dev_err(nandc->dev, >> + "failed to request slave channel\n"); >> + return -ENODEV; >> + } >> + } else { >> + nandc->reg_read_buf = >> + dmam_alloc_coherent(nandc->dev, MAX_REG_RD * >> + sizeof(*nandc->reg_read_buf), >> + &nandc->reg_read_buf_phys, >> + GFP_KERNEL); >> + >> + if (!nandc->reg_read_buf) >> + return -ENOMEM; >> + >> + nandc->tx_chan = dma_request_slave_channel(nandc->dev, "tx"); >> + if (!nandc->tx_chan) { >> + dev_err(nandc->dev, "failed to request tx channel\n"); >> + return -ENODEV; >> + } >> + >> + nandc->rx_chan = dma_request_slave_channel(nandc->dev, "rx"); >> + if (!nandc->rx_chan) { >> + dev_err(nandc->dev, "failed to request rx channel\n"); >> + return -ENODEV; >> + } >> + >> + nandc->cmd_chan = dma_request_slave_channel(nandc->dev, "cmd"); >> + if (!nandc->cmd_chan) { >> + dev_err(nandc->dev, "failed to request cmd channel\n"); >> + return -ENODEV; >> + } >> } >> >> INIT_LIST_HEAD(&nandc->desc_list); >> @@ -1978,7 +2022,19 @@ static int qcom_nandc_alloc(struct >> qcom_nand_controller *nandc) >> >> static void qcom_nandc_unalloc(struct qcom_nand_controller *nandc) >> { >> - dma_release_channel(nandc->chan); >> + if (nandc->dma_bam_enabled) { >> + if (nandc->tx_chan) >> + dma_release_channel(nandc->tx_chan); >> + >> + if (nandc->rx_chan) >> + dma_release_channel(nandc->rx_chan); >> + >> + if (nandc->cmd_chan) >> + dma_release_channel(nandc->cmd_chan); >> + } else { >> + if (nandc->chan) >> + dma_release_channel(nandc->chan); >> + } >> } >> >> /* one time setup of a few nand controller registers */ >> @@ -2063,16 +2119,20 @@ static int qcom_nandc_parse_dt(struct >> platform_device *pdev) >> struct device_node *np = nandc->dev->of_node; >> int ret; >> >> - ret = of_property_read_u32(np, "qcom,cmd-crci", &nandc->cmd_crci); >> - if (ret) { >> - dev_err(nandc->dev, "command CRCI unspecified\n"); >> - return ret; >> - } >> + if (!nandc->dma_bam_enabled) { >> + ret = of_property_read_u32(np, "qcom,cmd-crci", >> + &nandc->cmd_crci); >> + if (ret) { >> + dev_err(nandc->dev, "command CRCI unspecified\n"); >> + return ret; >> + } >> >> - ret = of_property_read_u32(np, "qcom,data-crci", &nandc->data_crci); >> - if (ret) { >> - dev_err(nandc->dev, "data CRCI unspecified\n"); >> - return ret; >> + ret = of_property_read_u32(np, "qcom,data-crci", >> + &nandc->data_crci); >> + if (ret) { >> + dev_err(nandc->dev, "data CRCI unspecified\n"); >> + return ret; >> + } >> } >> >> return 0; >> @@ -2128,7 +2188,7 @@ static int qcom_nandc_probe(struct >> platform_device *pdev) >> >> ret = qcom_nandc_alloc(nandc); >> if (ret) >> - return ret; >> + goto err_core_clk; >> >> ret = clk_prepare_enable(nandc->core_clk); >> if (ret) >>
On 2017-06-29 15:18, Marek Vasut wrote: > On 06/29/2017 09:15 AM, Abhishek Sahu wrote: >> 1. The QPIC NAND uses 3 BAM channels: command, data tx and >> data rx while EBI2 NAND uses only single ADM channel. >> >> 2. The EBI2 NAND uses normal register read buffer since this >> buffer will be remapped with dma_map_sg. The QPIC NAND will give >> register read buffer in command descriptor and the command >> descriptor will be mapped with dma_map_sg so the register buffer >> should be DMA coherent. >> >> Signed-off-by: Abhishek Sahu <absahu@codeaurora.org> > > The patch does two things, so make two patches. Also split the DT > changes into separate patch ... Sure. I will do the same in v2. > >> --- >> .../devicetree/bindings/mtd/qcom_nandc.txt | 25 +++-- >> drivers/mtd/nand/qcom_nandc.c | 106 >> ++++++++++++++++----- >> 2 files changed, 99 insertions(+), 32 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt >> b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt >> index 5d0f7ae..87b9a56 100644 >> --- a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt >> +++ b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt >> @@ -9,15 +9,17 @@ Required properties: >> - clock-names: must contain "core" for the core clock and "aon" for >> the >> always on clock >> - dmas: DMA specifier, consisting of a phandle to the ADM DMA >> - controller node and the channel number to be used for >> - NAND. Refer to dma.txt and qcom_adm.txt for more details >> -- dma-names: must be "rxtx" >> -- qcom,cmd-crci: must contain the ADM command type CRCI block >> instance >> - number specified for the NAND controller on the given >> - platform >> -- qcom,data-crci: must contain the ADM data type CRCI block instance >> - number specified for the NAND controller on the given >> - platform >> + or BAM DMA controller node and the channel number to >> + be used for NAND. Refer to dma.txt, qcom_adm.txt(ADM) >> + and qcom/bam_dma.txt(BAM) for more details >> +- dma-names: "rxtx" - ADM >> + "tx", "rx", "cmd" - BAM >> +- qcom,cmd-crci: Only required for ADM DMA. must contain the ADM >> command >> + type CRCI block instance number specified for the NAND >> + controller on the given platform. >> +- qcom,data-crci: Only required for ADM DMA. must contain the ADM >> data >> + type CRCI block instance number specified for the NAND >> + controller on the given platform. >> - #address-cells: <1> - subnodes give the chip-select number >> - #size-cells: <0> >> >> @@ -95,6 +97,11 @@ nand@79b0000 { >> <&gcc GCC_QPIC_AHB_CLK>; >> clock-names = "core", "aon"; >> >> + dmas = <&qpicbam 0>, >> + <&qpicbam 1>, >> + <&qpicbam 2>; >> + dma-names = "tx", "rx", "cmd"; >> + >> #address-cells = <1>; >> #size-cells = <0>; >> >> diff --git a/drivers/mtd/nand/qcom_nandc.c >> b/drivers/mtd/nand/qcom_nandc.c >> index f55f728..520add9 100644 >> --- a/drivers/mtd/nand/qcom_nandc.c >> +++ b/drivers/mtd/nand/qcom_nandc.c >> @@ -226,6 +226,7 @@ struct nandc_regs { >> * by upper layers directly >> * @buf_size/count/start: markers for chip->read_buf/write_buf >> functions >> * @reg_read_buf: local buffer for reading back registers via DMA >> + * @reg_read_buf_phys: contains dma address for register read buffer >> * @reg_read_pos: marker for data read in reg_read_buf >> * >> * @regs: a contiguous chunk of memory for DMA register >> @@ -249,9 +250,19 @@ struct qcom_nand_controller { >> struct clk *core_clk; >> struct clk *aon_clk; >> >> - struct dma_chan *chan; >> - unsigned int cmd_crci; >> - unsigned int data_crci; >> + union { >> + struct { >> + struct dma_chan *tx_chan; >> + struct dma_chan *rx_chan; >> + struct dma_chan *cmd_chan; >> + }; >> + struct { >> + struct dma_chan *chan; >> + unsigned int cmd_crci; >> + unsigned int data_crci; >> + }; >> + }; >> + >> struct list_head desc_list; >> >> u8 *data_buffer; >> @@ -261,6 +272,7 @@ struct qcom_nand_controller { >> int buf_start; >> >> __le32 *reg_read_buf; >> + dma_addr_t reg_read_buf_phys; >> int reg_read_pos; >> >> struct nandc_regs *regs; >> @@ -1956,16 +1968,48 @@ static int qcom_nandc_alloc(struct >> qcom_nand_controller *nandc) >> if (!nandc->regs) >> return -ENOMEM; >> >> - nandc->reg_read_buf = devm_kzalloc(nandc->dev, >> - MAX_REG_RD * sizeof(*nandc->reg_read_buf), >> - GFP_KERNEL); >> - if (!nandc->reg_read_buf) >> - return -ENOMEM; >> >> - nandc->chan = dma_request_slave_channel(nandc->dev, "rxtx"); >> - if (!nandc->chan) { >> - dev_err(nandc->dev, "failed to request slave channel\n"); >> - return -ENODEV; >> + if (!nandc->dma_bam_enabled) { >> + nandc->reg_read_buf = >> + devm_kzalloc(nandc->dev, MAX_REG_RD * >> + sizeof(*nandc->reg_read_buf), GFP_KERNEL); >> + >> + if (!nandc->reg_read_buf) >> + return -ENOMEM; >> + >> + nandc->chan = dma_request_slave_channel(nandc->dev, "rxtx"); >> + if (!nandc->chan) { >> + dev_err(nandc->dev, >> + "failed to request slave channel\n"); >> + return -ENODEV; >> + } >> + } else { >> + nandc->reg_read_buf = >> + dmam_alloc_coherent(nandc->dev, MAX_REG_RD * >> + sizeof(*nandc->reg_read_buf), >> + &nandc->reg_read_buf_phys, >> + GFP_KERNEL); >> + >> + if (!nandc->reg_read_buf) >> + return -ENOMEM; >> + >> + nandc->tx_chan = dma_request_slave_channel(nandc->dev, "tx"); >> + if (!nandc->tx_chan) { >> + dev_err(nandc->dev, "failed to request tx channel\n"); >> + return -ENODEV; >> + } >> + >> + nandc->rx_chan = dma_request_slave_channel(nandc->dev, "rx"); >> + if (!nandc->rx_chan) { >> + dev_err(nandc->dev, "failed to request rx channel\n"); >> + return -ENODEV; >> + } >> + >> + nandc->cmd_chan = dma_request_slave_channel(nandc->dev, "cmd"); >> + if (!nandc->cmd_chan) { >> + dev_err(nandc->dev, "failed to request cmd channel\n"); >> + return -ENODEV; >> + } >> } >> >> INIT_LIST_HEAD(&nandc->desc_list); >> @@ -1978,7 +2022,19 @@ static int qcom_nandc_alloc(struct >> qcom_nand_controller *nandc) >> >> static void qcom_nandc_unalloc(struct qcom_nand_controller *nandc) >> { >> - dma_release_channel(nandc->chan); >> + if (nandc->dma_bam_enabled) { >> + if (nandc->tx_chan) >> + dma_release_channel(nandc->tx_chan); >> + >> + if (nandc->rx_chan) >> + dma_release_channel(nandc->rx_chan); >> + >> + if (nandc->cmd_chan) >> + dma_release_channel(nandc->cmd_chan); >> + } else { >> + if (nandc->chan) >> + dma_release_channel(nandc->chan); >> + } >> } >> >> /* one time setup of a few nand controller registers */ >> @@ -2063,16 +2119,20 @@ static int qcom_nandc_parse_dt(struct >> platform_device *pdev) >> struct device_node *np = nandc->dev->of_node; >> int ret; >> >> - ret = of_property_read_u32(np, "qcom,cmd-crci", &nandc->cmd_crci); >> - if (ret) { >> - dev_err(nandc->dev, "command CRCI unspecified\n"); >> - return ret; >> - } >> + if (!nandc->dma_bam_enabled) { >> + ret = of_property_read_u32(np, "qcom,cmd-crci", >> + &nandc->cmd_crci); >> + if (ret) { >> + dev_err(nandc->dev, "command CRCI unspecified\n"); >> + return ret; >> + } >> >> - ret = of_property_read_u32(np, "qcom,data-crci", &nandc->data_crci); >> - if (ret) { >> - dev_err(nandc->dev, "data CRCI unspecified\n"); >> - return ret; >> + ret = of_property_read_u32(np, "qcom,data-crci", >> + &nandc->data_crci); >> + if (ret) { >> + dev_err(nandc->dev, "data CRCI unspecified\n"); >> + return ret; >> + } >> } >> >> return 0; >> @@ -2128,7 +2188,7 @@ static int qcom_nandc_probe(struct >> platform_device >> *pdev) >> >> ret = qcom_nandc_alloc(nandc); >> if (ret) >> - return ret; >> + goto err_core_clk; >> >> ret = clk_prepare_enable(nandc->core_clk); >> if (ret) >> > > Can you please fix your mailer to stop adding "QUALCOMM INDIA, on > behalf > of Qualcomm Innovation Center"... stuff at the bottom of the patches ? Sorry Marek. We can't remove this line since it is our legal team requirement and we need to follow this while submitting the patches. All the Qualcomm patches need be sent from codeaurora and this line implies that these patches are submitted from Qualcomm. this line will come only for the patches. For replying , this line will not come.
diff --git a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt index 5d0f7ae..87b9a56 100644 --- a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt +++ b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt @@ -9,15 +9,17 @@ Required properties: - clock-names: must contain "core" for the core clock and "aon" for the always on clock - dmas: DMA specifier, consisting of a phandle to the ADM DMA - controller node and the channel number to be used for - NAND. Refer to dma.txt and qcom_adm.txt for more details -- dma-names: must be "rxtx" -- qcom,cmd-crci: must contain the ADM command type CRCI block instance - number specified for the NAND controller on the given - platform -- qcom,data-crci: must contain the ADM data type CRCI block instance - number specified for the NAND controller on the given - platform + or BAM DMA controller node and the channel number to + be used for NAND. Refer to dma.txt, qcom_adm.txt(ADM) + and qcom/bam_dma.txt(BAM) for more details +- dma-names: "rxtx" - ADM + "tx", "rx", "cmd" - BAM +- qcom,cmd-crci: Only required for ADM DMA. must contain the ADM command + type CRCI block instance number specified for the NAND + controller on the given platform. +- qcom,data-crci: Only required for ADM DMA. must contain the ADM data + type CRCI block instance number specified for the NAND + controller on the given platform. - #address-cells: <1> - subnodes give the chip-select number - #size-cells: <0> @@ -95,6 +97,11 @@ nand@79b0000 { <&gcc GCC_QPIC_AHB_CLK>; clock-names = "core", "aon"; + dmas = <&qpicbam 0>, + <&qpicbam 1>, + <&qpicbam 2>; + dma-names = "tx", "rx", "cmd"; + #address-cells = <1>; #size-cells = <0>; diff --git a/drivers/mtd/nand/qcom_nandc.c b/drivers/mtd/nand/qcom_nandc.c index f55f728..520add9 100644 --- a/drivers/mtd/nand/qcom_nandc.c +++ b/drivers/mtd/nand/qcom_nandc.c @@ -226,6 +226,7 @@ struct nandc_regs { * by upper layers directly * @buf_size/count/start: markers for chip->read_buf/write_buf functions * @reg_read_buf: local buffer for reading back registers via DMA + * @reg_read_buf_phys: contains dma address for register read buffer * @reg_read_pos: marker for data read in reg_read_buf * * @regs: a contiguous chunk of memory for DMA register @@ -249,9 +250,19 @@ struct qcom_nand_controller { struct clk *core_clk; struct clk *aon_clk; - struct dma_chan *chan; - unsigned int cmd_crci; - unsigned int data_crci; + union { + struct { + struct dma_chan *tx_chan; + struct dma_chan *rx_chan; + struct dma_chan *cmd_chan; + }; + struct { + struct dma_chan *chan; + unsigned int cmd_crci; + unsigned int data_crci; + }; + }; + struct list_head desc_list; u8 *data_buffer; @@ -261,6 +272,7 @@ struct qcom_nand_controller { int buf_start; __le32 *reg_read_buf; + dma_addr_t reg_read_buf_phys; int reg_read_pos; struct nandc_regs *regs; @@ -1956,16 +1968,48 @@ static int qcom_nandc_alloc(struct qcom_nand_controller *nandc) if (!nandc->regs) return -ENOMEM; - nandc->reg_read_buf = devm_kzalloc(nandc->dev, - MAX_REG_RD * sizeof(*nandc->reg_read_buf), - GFP_KERNEL); - if (!nandc->reg_read_buf) - return -ENOMEM; - nandc->chan = dma_request_slave_channel(nandc->dev, "rxtx"); - if (!nandc->chan) { - dev_err(nandc->dev, "failed to request slave channel\n"); - return -ENODEV; + if (!nandc->dma_bam_enabled) { + nandc->reg_read_buf = + devm_kzalloc(nandc->dev, MAX_REG_RD * + sizeof(*nandc->reg_read_buf), GFP_KERNEL); + + if (!nandc->reg_read_buf) + return -ENOMEM; + + nandc->chan = dma_request_slave_channel(nandc->dev, "rxtx"); + if (!nandc->chan) { + dev_err(nandc->dev, + "failed to request slave channel\n"); + return -ENODEV; + } + } else { + nandc->reg_read_buf = + dmam_alloc_coherent(nandc->dev, MAX_REG_RD * + sizeof(*nandc->reg_read_buf), + &nandc->reg_read_buf_phys, + GFP_KERNEL); + + if (!nandc->reg_read_buf) + return -ENOMEM; + + nandc->tx_chan = dma_request_slave_channel(nandc->dev, "tx"); + if (!nandc->tx_chan) { + dev_err(nandc->dev, "failed to request tx channel\n"); + return -ENODEV; + } + + nandc->rx_chan = dma_request_slave_channel(nandc->dev, "rx"); + if (!nandc->rx_chan) { + dev_err(nandc->dev, "failed to request rx channel\n"); + return -ENODEV; + } + + nandc->cmd_chan = dma_request_slave_channel(nandc->dev, "cmd"); + if (!nandc->cmd_chan) { + dev_err(nandc->dev, "failed to request cmd channel\n"); + return -ENODEV; + } } INIT_LIST_HEAD(&nandc->desc_list); @@ -1978,7 +2022,19 @@ static int qcom_nandc_alloc(struct qcom_nand_controller *nandc) static void qcom_nandc_unalloc(struct qcom_nand_controller *nandc) { - dma_release_channel(nandc->chan); + if (nandc->dma_bam_enabled) { + if (nandc->tx_chan) + dma_release_channel(nandc->tx_chan); + + if (nandc->rx_chan) + dma_release_channel(nandc->rx_chan); + + if (nandc->cmd_chan) + dma_release_channel(nandc->cmd_chan); + } else { + if (nandc->chan) + dma_release_channel(nandc->chan); + } } /* one time setup of a few nand controller registers */ @@ -2063,16 +2119,20 @@ static int qcom_nandc_parse_dt(struct platform_device *pdev) struct device_node *np = nandc->dev->of_node; int ret; - ret = of_property_read_u32(np, "qcom,cmd-crci", &nandc->cmd_crci); - if (ret) { - dev_err(nandc->dev, "command CRCI unspecified\n"); - return ret; - } + if (!nandc->dma_bam_enabled) { + ret = of_property_read_u32(np, "qcom,cmd-crci", + &nandc->cmd_crci); + if (ret) { + dev_err(nandc->dev, "command CRCI unspecified\n"); + return ret; + } - ret = of_property_read_u32(np, "qcom,data-crci", &nandc->data_crci); - if (ret) { - dev_err(nandc->dev, "data CRCI unspecified\n"); - return ret; + ret = of_property_read_u32(np, "qcom,data-crci", + &nandc->data_crci); + if (ret) { + dev_err(nandc->dev, "data CRCI unspecified\n"); + return ret; + } } return 0; @@ -2128,7 +2188,7 @@ static int qcom_nandc_probe(struct platform_device *pdev) ret = qcom_nandc_alloc(nandc); if (ret) - return ret; + goto err_core_clk; ret = clk_prepare_enable(nandc->core_clk); if (ret)
1. The QPIC NAND uses 3 BAM channels: command, data tx and data rx while EBI2 NAND uses only single ADM channel. 2. The EBI2 NAND uses normal register read buffer since this buffer will be remapped with dma_map_sg. The QPIC NAND will give register read buffer in command descriptor and the command descriptor will be mapped with dma_map_sg so the register buffer should be DMA coherent. Signed-off-by: Abhishek Sahu <absahu@codeaurora.org> --- .../devicetree/bindings/mtd/qcom_nandc.txt | 25 +++-- drivers/mtd/nand/qcom_nandc.c | 106 ++++++++++++++++----- 2 files changed, 99 insertions(+), 32 deletions(-)