| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <f23d6ef150d1a4405301423b6be8d481@codeaurora.org>
Date: Mon, 17 Jul 2017 11:56:37 +0530
From: Abhishek Sahu <absahu@...eaurora.org>
To: Archit Taneja <architt@...eaurora.org>
Cc: dwmw2@...radead.org, computersforpeace@...il.com,
boris.brezillon@...e-electrons.com, marek.vasut@...il.com,
richard@....at, cyrille.pitchen@...ev4u.fr, robh+dt@...nel.org,
mark.rutland@....com, devicetree@...r.kernel.org,
linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-mtd@...ts.infradead.org, andy.gross@...aro.org,
sricharan@...eaurora.org
Subject: Re: [PATCH 02/14] qcom: mtd: nand: add and initialize QPIC DMA
resources
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@...eaurora.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@...0000 {
>> <&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)
>>
--
Abhishek Sahu
Powered by blists - more mailing lists