Re: [PATCH v2 2/5] mtd: nand: Qualcomm NAND controller driver
On 7/30/2015 12:03 AM, Stephen Boyd wrote:
On 07/29, Archit Taneja wrote:
On 07/29/2015 07:18 AM, Stephen Boyd wrote:
On 07/27/2015 09:34 PM, Archit Taneja wrote:
Hi,
On 07/25/2015 06:21 AM, Stephen Boyd wrote:
On 07/21/2015 03:34 AM, Archit Taneja wrote:
+ int size)
+{Looks like a
+struct desc_info *desc;
+struct dma_async_tx_descriptor *dma_desc;
+struct scatterlist *sgl;
+int r;
+
+desc = kzalloc(sizeof(*desc), GFP_KERNEL);
+if (!desc)
+return -ENOMEM;
+
+list_add_tail(>list, >list);
+
+sgl = >sgl;
+
+sg_init_one(sgl, vaddr, size);
+
+desc->dir = DMA_MEM_TO_DEV;
+
+r = dma_map_sg(this->dev, sgl, 1, desc->dir);
+if (r == 0)
+goto err;
Should we return an error in this case? Looks like return 0.
dma_map_sg returns the number of sg entries successfully mapped. In
this case, it should be 1.
Right, but this function returns 0 (success?) if we failed to map anything.
Yes. The return value is number of entries successfully mapped.
dma_map_sg is a macro that is replaced by dma_map_sg_attrs. Its
comment
says:
"dma_maps_sg_attrs returns 0 on error and > 0 on success. It should
never return a value < 0."
Yes, and so this function that calls dma_map_sg() is going to
return 0 to the caller when it didn't do what it was asked to do?
Ah, I get your point now :p. I thought you were referring to the
return value of dma_map_sg, and not write_data_dma. Will fix this.
Archit
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/5] mtd: nand: Qualcomm NAND controller driver
On 7/30/2015 12:03 AM, Stephen Boyd wrote:
On 07/29, Archit Taneja wrote:
On 07/29/2015 07:18 AM, Stephen Boyd wrote:
On 07/27/2015 09:34 PM, Archit Taneja wrote:
Hi,
On 07/25/2015 06:21 AM, Stephen Boyd wrote:
On 07/21/2015 03:34 AM, Archit Taneja wrote:
+ int size)
+{Looks like a
+struct desc_info *desc;
+struct dma_async_tx_descriptor *dma_desc;
+struct scatterlist *sgl;
+int r;
+
+desc = kzalloc(sizeof(*desc), GFP_KERNEL);
+if (!desc)
+return -ENOMEM;
+
+list_add_tail(desc-list, this-list);
+
+sgl = desc-sgl;
+
+sg_init_one(sgl, vaddr, size);
+
+desc-dir = DMA_MEM_TO_DEV;
+
+r = dma_map_sg(this-dev, sgl, 1, desc-dir);
+if (r == 0)
+goto err;
Should we return an error in this case? Looks like return 0.
dma_map_sg returns the number of sg entries successfully mapped. In
this case, it should be 1.
Right, but this function returns 0 (success?) if we failed to map anything.
Yes. The return value is number of entries successfully mapped.
dma_map_sg is a macro that is replaced by dma_map_sg_attrs. Its
comment
says:
dma_maps_sg_attrs returns 0 on error and 0 on success. It should
never return a value 0.
Yes, and so this function that calls dma_map_sg() is going to
return 0 to the caller when it didn't do what it was asked to do?
Ah, I get your point now :p. I thought you were referring to the
return value of dma_map_sg, and not write_data_dma. Will fix this.
Archit
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/5] mtd: nand: Qualcomm NAND controller driver
On 07/29, Archit Taneja wrote:
> On 07/29/2015 07:18 AM, Stephen Boyd wrote:
> >On 07/27/2015 09:34 PM, Archit Taneja wrote:
> >>Hi,
> >>
> >>On 07/25/2015 06:21 AM, Stephen Boyd wrote:
> >>>On 07/21/2015 03:34 AM, Archit Taneja wrote:
> >>>
> + int size)
> +{Looks like a
> +struct desc_info *desc;
> +struct dma_async_tx_descriptor *dma_desc;
> +struct scatterlist *sgl;
> +int r;
> +
> +desc = kzalloc(sizeof(*desc), GFP_KERNEL);
> +if (!desc)
> +return -ENOMEM;
> +
> +list_add_tail(>list, >list);
> +
> +sgl = >sgl;
> +
> +sg_init_one(sgl, vaddr, size);
> +
> +desc->dir = DMA_MEM_TO_DEV;
> +
> +r = dma_map_sg(this->dev, sgl, 1, desc->dir);
> +if (r == 0)
> +goto err;
> >>>
> >>>Should we return an error in this case? Looks like return 0.
> >>
> >>dma_map_sg returns the number of sg entries successfully mapped. In
> >>this case, it should be 1.
> >
> >Right, but this function returns 0 (success?) if we failed to map anything.
>
> Yes. The return value is number of entries successfully mapped.
> dma_map_sg is a macro that is replaced by dma_map_sg_attrs. Its
> comment
> says:
>
> "dma_maps_sg_attrs returns 0 on error and > 0 on success. It should
> never return a value < 0."
Yes, and so this function that calls dma_map_sg() is going to
return 0 to the caller when it didn't do what it was asked to do?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/5] mtd: nand: Qualcomm NAND controller driver
On 07/29, Archit Taneja wrote:
On 07/29/2015 07:18 AM, Stephen Boyd wrote:
On 07/27/2015 09:34 PM, Archit Taneja wrote:
Hi,
On 07/25/2015 06:21 AM, Stephen Boyd wrote:
On 07/21/2015 03:34 AM, Archit Taneja wrote:
+ int size)
+{Looks like a
+struct desc_info *desc;
+struct dma_async_tx_descriptor *dma_desc;
+struct scatterlist *sgl;
+int r;
+
+desc = kzalloc(sizeof(*desc), GFP_KERNEL);
+if (!desc)
+return -ENOMEM;
+
+list_add_tail(desc-list, this-list);
+
+sgl = desc-sgl;
+
+sg_init_one(sgl, vaddr, size);
+
+desc-dir = DMA_MEM_TO_DEV;
+
+r = dma_map_sg(this-dev, sgl, 1, desc-dir);
+if (r == 0)
+goto err;
Should we return an error in this case? Looks like return 0.
dma_map_sg returns the number of sg entries successfully mapped. In
this case, it should be 1.
Right, but this function returns 0 (success?) if we failed to map anything.
Yes. The return value is number of entries successfully mapped.
dma_map_sg is a macro that is replaced by dma_map_sg_attrs. Its
comment
says:
dma_maps_sg_attrs returns 0 on error and 0 on success. It should
never return a value 0.
Yes, and so this function that calls dma_map_sg() is going to
return 0 to the caller when it didn't do what it was asked to do?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/5] mtd: nand: Qualcomm NAND controller driver
On 07/29/2015 07:18 AM, Stephen Boyd wrote:
On 07/27/2015 09:34 PM, Archit Taneja wrote:
Hi,
On 07/25/2015 06:21 AM, Stephen Boyd wrote:
On 07/21/2015 03:34 AM, Archit Taneja wrote:
+ int size)
+{Looks like a
+struct desc_info *desc;
+struct dma_async_tx_descriptor *dma_desc;
+struct scatterlist *sgl;
+int r;
+
+desc = kzalloc(sizeof(*desc), GFP_KERNEL);
+if (!desc)
+return -ENOMEM;
+
+list_add_tail(>list, >list);
+
+sgl = >sgl;
+
+sg_init_one(sgl, vaddr, size);
+
+desc->dir = DMA_MEM_TO_DEV;
+
+r = dma_map_sg(this->dev, sgl, 1, desc->dir);
+if (r == 0)
+goto err;
Should we return an error in this case? Looks like return 0.
dma_map_sg returns the number of sg entries successfully mapped. In
this case, it should be 1.
Right, but this function returns 0 (success?) if we failed to map anything.
Yes. The return value is number of entries successfully mapped.
dma_map_sg is a macro that is replaced by dma_map_sg_attrs. Its comment
says:
"dma_maps_sg_attrs returns 0 on error and > 0 on success. It should
never return a value < 0."
+
+this->slave_conf.device_fc = 0;
+this->slave_conf.dst_addr = this->res->start + reg_off;
+this->slave_conf.dst_maxburst = 16;
Is there any reason why slave_conf can't be on the stack? Otherwise it's
odd that it's overwritten a few times before we submit the descriptors,
so it must be copied by the dmaengine provider, but that isn't clear at
all from the code. If it isn't copied, perhaps it should be part of the
desc_info structure. If it is copied I wonder why it isn't const in the
function signature.
The dmaengine drivers either memcpy slave_config in their
device_config() dmaengine op, or populate their local members reading
params in the passed slave_config.
I'll move slave_conf to stack. As you said, the config argument
in dmaengine_slave_config should ideally use const.
Cool, someone should send a patch.
+
+r = dmaengine_slave_config(this->chan, >slave_conf);
+if (r) {
+dev_err(this->dev, "failed to configure dma channel\n");
+goto err;
+}
+
+dma_desc = dmaengine_prep_slave_sg(this->chan, sgl, 1,
desc->dir, 0);
+if (!dma_desc) {
+dev_err(this->dev, "failed to prepare data write desc\n");
+r = PTR_ERR(dma_desc);
+goto err;
+}
+
+desc->dma_desc = dma_desc;
+
+return 0;
+err:
+kfree(desc);
+
+return r;
+}
+
+/*
+ * helper to prepare dma descriptors to configure registers needed
for reading a
+ * codeword/step in a page
+ */
+static void config_cw_read(struct qcom_nandc_data *this)
+{
+struct nandc_regs *regs = this->regs;
+
+write_reg_dma(this, NAND_FLASH_CMD, >cmd, 3, true);
Maybe it would be better to have a case statement inside
{write,read}_reg_dma() that looked at the second argument and matched it
up with an offset in regs. Then this could be
write_reg_dma(this, NAND_FLASH_CMD, 3, true);
That's a good idea. However, we have at least one programming seqeunce
(in nandc_param) where we need to write two different values to the
same register. In such a case, we need two different locations to
store the two values.
I can split up the programming sequence into two parts such that we
won't write to the same register twice. But doing this for the sake of
reducing an argument to write_reg_dma seems a bit unnecessary.
Or you could have two #defines that indicate the different usage? Like
NAND_CMD_FOO and NAND_CMD_BAR that both map to the same register but
uses different locations as storage.
Yeah, that seems like a good option. I'll try this out.
Are we guaranteed that this is called within the same context as where
the buffer is passed to this function? Otherwise this stack check isn't
going to work because object_is_on_stack() will silently fail.
These are funcs are finally tied to mtd ops. I think in normal operation
it'll be the same context. I'll still cross check. The aim of the check
is to prevent a memcpy of the buffer to/from the layer above. A false
negative will result in a slower read/write operation.
Right. It would be nice if the mtd layer was DMA aware and could
indicate to drivers that DMA on the buffer is allowable or not. Trying
to figure it out if the buffer is in lowmem after the buffer is mapped
is error prone, which is why not a lot of usage of object_is_on_stack()
exists. Honestly I'm confused, I thought the DMA APIs would "do the
right thing" when highmem was passed into the mapping APIs, but maybe
I'm wrong. I'll have to look.
It looks like NAND_USE_BOUNCE_BUFFER does just that. If we set this
flag, the nand_base layer provides a DMA-able buffer even if the
filesystem didn't.
No one was using this flag when I last checked. A new driver
brcmnand now uses it. I'll give this a try.
Archit
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from
Re: [PATCH v2 2/5] mtd: nand: Qualcomm NAND controller driver
On 07/27/2015 09:34 PM, Archit Taneja wrote:
Hi,
On 07/25/2015 06:21 AM, Stephen Boyd wrote:
On 07/21/2015 03:34 AM, Archit Taneja wrote:
+ int size)
+{
+struct desc_info *desc;
+struct dma_async_tx_descriptor *dma_desc;
+struct scatterlist *sgl;
+int r;
+
+desc = kzalloc(sizeof(*desc), GFP_KERNEL);
+if (!desc)
+return -ENOMEM;
+
+list_add_tail(>list, >list);
+
+sgl = >sgl;
+
+sg_init_one(sgl, vaddr, size);
+
+desc->dir = DMA_MEM_TO_DEV;
+
+r = dma_map_sg(this->dev, sgl, 1, desc->dir);
+if (r == 0)
+goto err;
Should we return an error in this case? Looks like return 0.
dma_map_sg returns the number of sg entries successfully mapped. In
this case, it should be 1.
Right, but this function returns 0 (success?) if we failed to map anything.
+
+this->slave_conf.device_fc = 0;
+this->slave_conf.dst_addr = this->res->start + reg_off;
+this->slave_conf.dst_maxburst = 16;
Is there any reason why slave_conf can't be on the stack? Otherwise it's
odd that it's overwritten a few times before we submit the descriptors,
so it must be copied by the dmaengine provider, but that isn't clear at
all from the code. If it isn't copied, perhaps it should be part of the
desc_info structure. If it is copied I wonder why it isn't const in the
function signature.
The dmaengine drivers either memcpy slave_config in their
device_config() dmaengine op, or populate their local members reading
params in the passed slave_config.
I'll move slave_conf to stack. As you said, the config argument
in dmaengine_slave_config should ideally use const.
Cool, someone should send a patch.
+
+r = dmaengine_slave_config(this->chan, >slave_conf);
+if (r) {
+dev_err(this->dev, "failed to configure dma channel\n");
+goto err;
+}
+
+dma_desc = dmaengine_prep_slave_sg(this->chan, sgl, 1,
desc->dir, 0);
+if (!dma_desc) {
+dev_err(this->dev, "failed to prepare data write desc\n");
+r = PTR_ERR(dma_desc);
+goto err;
+}
+
+desc->dma_desc = dma_desc;
+
+return 0;
+err:
+kfree(desc);
+
+return r;
+}
+
+/*
+ * helper to prepare dma descriptors to configure registers needed
for reading a
+ * codeword/step in a page
+ */
+static void config_cw_read(struct qcom_nandc_data *this)
+{
+struct nandc_regs *regs = this->regs;
+
+write_reg_dma(this, NAND_FLASH_CMD, >cmd, 3, true);
Maybe it would be better to have a case statement inside
{write,read}_reg_dma() that looked at the second argument and matched it
up with an offset in regs. Then this could be
write_reg_dma(this, NAND_FLASH_CMD, 3, true);
That's a good idea. However, we have at least one programming seqeunce
(in nandc_param) where we need to write two different values to the
same register. In such a case, we need two different locations to
store the two values.
I can split up the programming sequence into two parts such that we
won't write to the same register twice. But doing this for the sake of
reducing an argument to write_reg_dma seems a bit unnecessary.
Or you could have two #defines that indicate the different usage? Like
NAND_CMD_FOO and NAND_CMD_BAR that both map to the same register but
uses different locations as storage.
Are we guaranteed that this is called within the same context as where
the buffer is passed to this function? Otherwise this stack check isn't
going to work because object_is_on_stack() will silently fail.
These are funcs are finally tied to mtd ops. I think in normal operation
it'll be the same context. I'll still cross check. The aim of the check
is to prevent a memcpy of the buffer to/from the layer above. A false
negative will result in a slower read/write operation.
Right. It would be nice if the mtd layer was DMA aware and could
indicate to drivers that DMA on the buffer is allowable or not. Trying
to figure it out if the buffer is in lowmem after the buffer is mapped
is error prone, which is why not a lot of usage of object_is_on_stack()
exists. Honestly I'm confused, I thought the DMA APIs would "do the
right thing" when highmem was passed into the mapping APIs, but maybe
I'm wrong. I'll have to look.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/5] mtd: nand: Qualcomm NAND controller driver
On 07/27/2015 09:34 PM, Archit Taneja wrote:
Hi,
On 07/25/2015 06:21 AM, Stephen Boyd wrote:
On 07/21/2015 03:34 AM, Archit Taneja wrote:
+ int size)
+{
+struct desc_info *desc;
+struct dma_async_tx_descriptor *dma_desc;
+struct scatterlist *sgl;
+int r;
+
+desc = kzalloc(sizeof(*desc), GFP_KERNEL);
+if (!desc)
+return -ENOMEM;
+
+list_add_tail(desc-list, this-list);
+
+sgl = desc-sgl;
+
+sg_init_one(sgl, vaddr, size);
+
+desc-dir = DMA_MEM_TO_DEV;
+
+r = dma_map_sg(this-dev, sgl, 1, desc-dir);
+if (r == 0)
+goto err;
Should we return an error in this case? Looks like return 0.
dma_map_sg returns the number of sg entries successfully mapped. In
this case, it should be 1.
Right, but this function returns 0 (success?) if we failed to map anything.
+
+this-slave_conf.device_fc = 0;
+this-slave_conf.dst_addr = this-res-start + reg_off;
+this-slave_conf.dst_maxburst = 16;
Is there any reason why slave_conf can't be on the stack? Otherwise it's
odd that it's overwritten a few times before we submit the descriptors,
so it must be copied by the dmaengine provider, but that isn't clear at
all from the code. If it isn't copied, perhaps it should be part of the
desc_info structure. If it is copied I wonder why it isn't const in the
function signature.
The dmaengine drivers either memcpy slave_config in their
device_config() dmaengine op, or populate their local members reading
params in the passed slave_config.
I'll move slave_conf to stack. As you said, the config argument
in dmaengine_slave_config should ideally use const.
Cool, someone should send a patch.
+
+r = dmaengine_slave_config(this-chan, this-slave_conf);
+if (r) {
+dev_err(this-dev, failed to configure dma channel\n);
+goto err;
+}
+
+dma_desc = dmaengine_prep_slave_sg(this-chan, sgl, 1,
desc-dir, 0);
+if (!dma_desc) {
+dev_err(this-dev, failed to prepare data write desc\n);
+r = PTR_ERR(dma_desc);
+goto err;
+}
+
+desc-dma_desc = dma_desc;
+
+return 0;
+err:
+kfree(desc);
+
+return r;
+}
+
+/*
+ * helper to prepare dma descriptors to configure registers needed
for reading a
+ * codeword/step in a page
+ */
+static void config_cw_read(struct qcom_nandc_data *this)
+{
+struct nandc_regs *regs = this-regs;
+
+write_reg_dma(this, NAND_FLASH_CMD, regs-cmd, 3, true);
Maybe it would be better to have a case statement inside
{write,read}_reg_dma() that looked at the second argument and matched it
up with an offset in regs. Then this could be
write_reg_dma(this, NAND_FLASH_CMD, 3, true);
That's a good idea. However, we have at least one programming seqeunce
(in nandc_param) where we need to write two different values to the
same register. In such a case, we need two different locations to
store the two values.
I can split up the programming sequence into two parts such that we
won't write to the same register twice. But doing this for the sake of
reducing an argument to write_reg_dma seems a bit unnecessary.
Or you could have two #defines that indicate the different usage? Like
NAND_CMD_FOO and NAND_CMD_BAR that both map to the same register but
uses different locations as storage.
Are we guaranteed that this is called within the same context as where
the buffer is passed to this function? Otherwise this stack check isn't
going to work because object_is_on_stack() will silently fail.
These are funcs are finally tied to mtd ops. I think in normal operation
it'll be the same context. I'll still cross check. The aim of the check
is to prevent a memcpy of the buffer to/from the layer above. A false
negative will result in a slower read/write operation.
Right. It would be nice if the mtd layer was DMA aware and could
indicate to drivers that DMA on the buffer is allowable or not. Trying
to figure it out if the buffer is in lowmem after the buffer is mapped
is error prone, which is why not a lot of usage of object_is_on_stack()
exists. Honestly I'm confused, I thought the DMA APIs would do the
right thing when highmem was passed into the mapping APIs, but maybe
I'm wrong. I'll have to look.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/5] mtd: nand: Qualcomm NAND controller driver
On 07/29/2015 07:18 AM, Stephen Boyd wrote:
On 07/27/2015 09:34 PM, Archit Taneja wrote:
Hi,
On 07/25/2015 06:21 AM, Stephen Boyd wrote:
On 07/21/2015 03:34 AM, Archit Taneja wrote:
+ int size)
+{Looks like a
+struct desc_info *desc;
+struct dma_async_tx_descriptor *dma_desc;
+struct scatterlist *sgl;
+int r;
+
+desc = kzalloc(sizeof(*desc), GFP_KERNEL);
+if (!desc)
+return -ENOMEM;
+
+list_add_tail(desc-list, this-list);
+
+sgl = desc-sgl;
+
+sg_init_one(sgl, vaddr, size);
+
+desc-dir = DMA_MEM_TO_DEV;
+
+r = dma_map_sg(this-dev, sgl, 1, desc-dir);
+if (r == 0)
+goto err;
Should we return an error in this case? Looks like return 0.
dma_map_sg returns the number of sg entries successfully mapped. In
this case, it should be 1.
Right, but this function returns 0 (success?) if we failed to map anything.
Yes. The return value is number of entries successfully mapped.
dma_map_sg is a macro that is replaced by dma_map_sg_attrs. Its comment
says:
dma_maps_sg_attrs returns 0 on error and 0 on success. It should
never return a value 0.
+
+this-slave_conf.device_fc = 0;
+this-slave_conf.dst_addr = this-res-start + reg_off;
+this-slave_conf.dst_maxburst = 16;
Is there any reason why slave_conf can't be on the stack? Otherwise it's
odd that it's overwritten a few times before we submit the descriptors,
so it must be copied by the dmaengine provider, but that isn't clear at
all from the code. If it isn't copied, perhaps it should be part of the
desc_info structure. If it is copied I wonder why it isn't const in the
function signature.
The dmaengine drivers either memcpy slave_config in their
device_config() dmaengine op, or populate their local members reading
params in the passed slave_config.
I'll move slave_conf to stack. As you said, the config argument
in dmaengine_slave_config should ideally use const.
Cool, someone should send a patch.
+
+r = dmaengine_slave_config(this-chan, this-slave_conf);
+if (r) {
+dev_err(this-dev, failed to configure dma channel\n);
+goto err;
+}
+
+dma_desc = dmaengine_prep_slave_sg(this-chan, sgl, 1,
desc-dir, 0);
+if (!dma_desc) {
+dev_err(this-dev, failed to prepare data write desc\n);
+r = PTR_ERR(dma_desc);
+goto err;
+}
+
+desc-dma_desc = dma_desc;
+
+return 0;
+err:
+kfree(desc);
+
+return r;
+}
+
+/*
+ * helper to prepare dma descriptors to configure registers needed
for reading a
+ * codeword/step in a page
+ */
+static void config_cw_read(struct qcom_nandc_data *this)
+{
+struct nandc_regs *regs = this-regs;
+
+write_reg_dma(this, NAND_FLASH_CMD, regs-cmd, 3, true);
Maybe it would be better to have a case statement inside
{write,read}_reg_dma() that looked at the second argument and matched it
up with an offset in regs. Then this could be
write_reg_dma(this, NAND_FLASH_CMD, 3, true);
That's a good idea. However, we have at least one programming seqeunce
(in nandc_param) where we need to write two different values to the
same register. In such a case, we need two different locations to
store the two values.
I can split up the programming sequence into two parts such that we
won't write to the same register twice. But doing this for the sake of
reducing an argument to write_reg_dma seems a bit unnecessary.
Or you could have two #defines that indicate the different usage? Like
NAND_CMD_FOO and NAND_CMD_BAR that both map to the same register but
uses different locations as storage.
Yeah, that seems like a good option. I'll try this out.
Are we guaranteed that this is called within the same context as where
the buffer is passed to this function? Otherwise this stack check isn't
going to work because object_is_on_stack() will silently fail.
These are funcs are finally tied to mtd ops. I think in normal operation
it'll be the same context. I'll still cross check. The aim of the check
is to prevent a memcpy of the buffer to/from the layer above. A false
negative will result in a slower read/write operation.
Right. It would be nice if the mtd layer was DMA aware and could
indicate to drivers that DMA on the buffer is allowable or not. Trying
to figure it out if the buffer is in lowmem after the buffer is mapped
is error prone, which is why not a lot of usage of object_is_on_stack()
exists. Honestly I'm confused, I thought the DMA APIs would do the
right thing when highmem was passed into the mapping APIs, but maybe
I'm wrong. I'll have to look.
It looks like NAND_USE_BOUNCE_BUFFER does just that. If we set this
flag, the nand_base layer provides a DMA-able buffer even if the
filesystem didn't.
No one was using this flag when I last checked. A new driver
brcmnand now uses it. I'll give this a try.
Archit
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this
Re: [PATCH v2 2/5] mtd: nand: Qualcomm NAND controller driver
Hi,
On 07/25/2015 06:21 AM, Stephen Boyd wrote:
On 07/21/2015 03:34 AM, Archit Taneja wrote:
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 5b2806a..31951fc 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -538,4 +538,11 @@ config MTD_NAND_HISI504
help
Enables support for NAND controller on Hisilicon SoC Hip04.
+config MTD_NAND_QCOM
+ tristate "Support for NAND on QCOM SoCs"
+ depends on ARCH_QCOM && QCOM_ADM
This is sort of annoying that the menu won't show up unless the ADM
driver is also enabled (which would be in a completely different area of
the configurator). Perhaps drop that requirement because it isn't
required to build?
That makes sense. I'll drop the QCOM_ADM requirement.
+ help
+ Enables support for NAND flash chips on SoCs containing the EBI2 NAND
+ controller. This controller is found on IPQ806x SoC.
+
endif # MTD_NAND
diff --git a/drivers/mtd/nand/qcom_nandc.c b/drivers/mtd/nand/qcom_nandc.c
new file mode 100644
index 000..51c284c
--- /dev/null
+++ b/drivers/mtd/nand/qcom_nandc.c
@@ -0,0 +1,2019 @@
+/*
+ * Copyright (c) 2015, The Linux Foundation. All rights reserved.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include
+#include
+#include
Where is this used?
It isn't. I'll remove it.
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
[..]
+/*
+ * the NAND controller performs reads/writes with ECC in 516 byte chunks.
+ * the driver calls the chunks 'step' or 'codeword' interchangeably
+ */
+#define NANDC_STEP_SIZE512
+
+/*
+ * the largest page size we support is 8K, this will have 16 steps/codewords
+ * of 512 bytes each
+ */
+#defineMAX_NUM_STEPS (SZ_8K / NANDC_STEP_SIZE)
+
+/* we read at most 3 registers per codeword scan */
+#define MAX_REG_RD (3 * MAX_NUM_STEPS)
+
+/* ECC modes */
+#define ECC_NONE BIT(0)
+#define ECC_RS_4BITBIT(1)
+#defineECC_BCH_4BITBIT(2)
+#defineECC_BCH_8BITBIT(3)
+
+struct desc_info {
+ struct list_head list;
+
+ enum dma_transfer_direction dir;
+ struct scatterlist sgl;
+ struct dma_async_tx_descriptor *dma_desc;
+};
+
+/*
+ * holds the current register values that we want to write. acts as a
contiguous
+ * chunk of memory which we use to write the controller registers through DMA.
+ */
+struct nandc_regs {
+ u32 cmd;
+ u32 addr0;
+ u32 addr1;
+ u32 chip_sel;
+ u32 exec;
+
+ u32 cfg0;
+ u32 cfg1;
+ u32 ecc_bch_cfg;
+
+ u32 clrflashstatus;
+ u32 clrreadstatus;
+
+ u32 cmd1;
+ u32 vld;
+
+ u32 orig_cmd1;
+ u32 orig_vld;
+
+ u32 ecc_buf_cfg;
+};
+
+/*
+ * @cmd_crci: ADM DMA CRCI for command flow control
+ * @data_crci: ADM DMA CRCI for data flow control
+ * @list: DMA descriptor list (list of desc_infos)
+ * @dma_done: completion param to denote end of last
+ * descriptor in the list
+ * @data_buffer: our local DMA buffer for page read/writes,
+ * used when we can't use the buffer provided
+ * by upper layers directly
+ * @buf_size/count/start: markers for chip->read_buf/write_buf functions
+ * @reg_read_buf: buffer for reading register data via DMA
+ * @reg_read_pos: marker for data read in reg_read_buf
+ * @cfg0, cfg1, cfg0_raw..:NANDc register configurations needed for
+ * ecc/non-ecc mode for the current nand flash
+ * device
+ * @regs: a contiguous chunk of memory for DMA register
+ * writes
+ * @ecc_strength: 4 bit or 8 bit ecc, received via DT
+ * @bus_width: 8 bit or 16 bit NAND bus width, received via DT
+ * @ecc_modes: supported ECC modes by the current controller,
+ * initialized via DT match data
+ * @cw_size: the number of bytes in a single step/codeword
+ * of a page, consisting of all data, ecc, spare
+ * and reserved bytes
+ * @cw_data: the number of bytes within a codeword protected
+ * by ECC
+ * @bch_enabled: flag
Re: [PATCH v2 2/5] mtd: nand: Qualcomm NAND controller driver
Hi,
On 07/25/2015 06:21 AM, Stephen Boyd wrote:
On 07/21/2015 03:34 AM, Archit Taneja wrote:
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 5b2806a..31951fc 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -538,4 +538,11 @@ config MTD_NAND_HISI504
help
Enables support for NAND controller on Hisilicon SoC Hip04.
+config MTD_NAND_QCOM
+ tristate Support for NAND on QCOM SoCs
+ depends on ARCH_QCOM QCOM_ADM
This is sort of annoying that the menu won't show up unless the ADM
driver is also enabled (which would be in a completely different area of
the configurator). Perhaps drop that requirement because it isn't
required to build?
That makes sense. I'll drop the QCOM_ADM requirement.
+ help
+ Enables support for NAND flash chips on SoCs containing the EBI2 NAND
+ controller. This controller is found on IPQ806x SoC.
+
endif # MTD_NAND
diff --git a/drivers/mtd/nand/qcom_nandc.c b/drivers/mtd/nand/qcom_nandc.c
new file mode 100644
index 000..51c284c
--- /dev/null
+++ b/drivers/mtd/nand/qcom_nandc.c
@@ -0,0 +1,2019 @@
+/*
+ * Copyright (c) 2015, The Linux Foundation. All rights reserved.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include linux/clk.h
+#include linux/slab.h
+#include linux/interrupt.h
Where is this used?
It isn't. I'll remove it.
+#include linux/bitops.h
+#include linux/dma-mapping.h
+#include linux/dmaengine.h
+#include linux/module.h
+#include linux/mtd/nand.h
+#include linux/mtd/partitions.h
+#include linux/of.h
+#include linux/of_device.h
+#include linux/of_mtd.h
+#include linux/delay.h
+
[..]
+/*
+ * the NAND controller performs reads/writes with ECC in 516 byte chunks.
+ * the driver calls the chunks 'step' or 'codeword' interchangeably
+ */
+#define NANDC_STEP_SIZE512
+
+/*
+ * the largest page size we support is 8K, this will have 16 steps/codewords
+ * of 512 bytes each
+ */
+#defineMAX_NUM_STEPS (SZ_8K / NANDC_STEP_SIZE)
+
+/* we read at most 3 registers per codeword scan */
+#define MAX_REG_RD (3 * MAX_NUM_STEPS)
+
+/* ECC modes */
+#define ECC_NONE BIT(0)
+#define ECC_RS_4BITBIT(1)
+#defineECC_BCH_4BITBIT(2)
+#defineECC_BCH_8BITBIT(3)
+
+struct desc_info {
+ struct list_head list;
+
+ enum dma_transfer_direction dir;
+ struct scatterlist sgl;
+ struct dma_async_tx_descriptor *dma_desc;
+};
+
+/*
+ * holds the current register values that we want to write. acts as a
contiguous
+ * chunk of memory which we use to write the controller registers through DMA.
+ */
+struct nandc_regs {
+ u32 cmd;
+ u32 addr0;
+ u32 addr1;
+ u32 chip_sel;
+ u32 exec;
+
+ u32 cfg0;
+ u32 cfg1;
+ u32 ecc_bch_cfg;
+
+ u32 clrflashstatus;
+ u32 clrreadstatus;
+
+ u32 cmd1;
+ u32 vld;
+
+ u32 orig_cmd1;
+ u32 orig_vld;
+
+ u32 ecc_buf_cfg;
+};
+
+/*
+ * @cmd_crci: ADM DMA CRCI for command flow control
+ * @data_crci: ADM DMA CRCI for data flow control
+ * @list: DMA descriptor list (list of desc_infos)
+ * @dma_done: completion param to denote end of last
+ * descriptor in the list
+ * @data_buffer: our local DMA buffer for page read/writes,
+ * used when we can't use the buffer provided
+ * by upper layers directly
+ * @buf_size/count/start: markers for chip-read_buf/write_buf functions
+ * @reg_read_buf: buffer for reading register data via DMA
+ * @reg_read_pos: marker for data read in reg_read_buf
+ * @cfg0, cfg1, cfg0_raw..:NANDc register configurations needed for
+ * ecc/non-ecc mode for the current nand flash
+ * device
+ * @regs: a contiguous chunk of memory for DMA register
+ * writes
+ * @ecc_strength: 4 bit or 8 bit ecc, received via DT
+ * @bus_width: 8 bit or 16 bit NAND bus width, received via DT
+ * @ecc_modes: supported ECC modes by the current controller,
+ * initialized via DT match data
+ * @cw_size: the number of bytes in a single step/codeword
+ * of a page, consisting of all data, ecc, spare
+ *
Re: [PATCH v2 2/5] mtd: nand: Qualcomm NAND controller driver
On 07/21/2015 03:34 AM, Archit Taneja wrote:
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index 5b2806a..31951fc 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -538,4 +538,11 @@ config MTD_NAND_HISI504
> help
> Enables support for NAND controller on Hisilicon SoC Hip04.
>
> +config MTD_NAND_QCOM
> + tristate "Support for NAND on QCOM SoCs"
> + depends on ARCH_QCOM && QCOM_ADM
This is sort of annoying that the menu won't show up unless the ADM
driver is also enabled (which would be in a completely different area of
the configurator). Perhaps drop that requirement because it isn't
required to build?
> + help
> + Enables support for NAND flash chips on SoCs containing the EBI2 NAND
> + controller. This controller is found on IPQ806x SoC.
> +
> endif # MTD_NAND
> diff --git a/drivers/mtd/nand/qcom_nandc.c b/drivers/mtd/nand/qcom_nandc.c
> new file mode 100644
> index 000..51c284c
> --- /dev/null
> +++ b/drivers/mtd/nand/qcom_nandc.c
> @@ -0,0 +1,2019 @@
> +/*
> + * Copyright (c) 2015, The Linux Foundation. All rights reserved.
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include
> +#include
> +#include
Where is this used?
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
[..]
> +/*
> + * the NAND controller performs reads/writes with ECC in 516 byte chunks.
> + * the driver calls the chunks 'step' or 'codeword' interchangeably
> + */
> +#define NANDC_STEP_SIZE 512
> +
> +/*
> + * the largest page size we support is 8K, this will have 16 steps/codewords
> + * of 512 bytes each
> + */
> +#define MAX_NUM_STEPS (SZ_8K / NANDC_STEP_SIZE)
> +
> +/* we read at most 3 registers per codeword scan */
> +#define MAX_REG_RD (3 * MAX_NUM_STEPS)
> +
> +/* ECC modes */
> +#define ECC_NONE BIT(0)
> +#define ECC_RS_4BIT BIT(1)
> +#define ECC_BCH_4BITBIT(2)
> +#define ECC_BCH_8BITBIT(3)
> +
> +struct desc_info {
> + struct list_head list;
> +
> + enum dma_transfer_direction dir;
> + struct scatterlist sgl;
> + struct dma_async_tx_descriptor *dma_desc;
> +};
> +
> +/*
> + * holds the current register values that we want to write. acts as a
> contiguous
> + * chunk of memory which we use to write the controller registers through
> DMA.
> + */
> +struct nandc_regs {
> + u32 cmd;
> + u32 addr0;
> + u32 addr1;
> + u32 chip_sel;
> + u32 exec;
> +
> + u32 cfg0;
> + u32 cfg1;
> + u32 ecc_bch_cfg;
> +
> + u32 clrflashstatus;
> + u32 clrreadstatus;
> +
> + u32 cmd1;
> + u32 vld;
> +
> + u32 orig_cmd1;
> + u32 orig_vld;
> +
> + u32 ecc_buf_cfg;
> +};
> +
> +/*
> + * @cmd_crci:ADM DMA CRCI for command flow control
> + * @data_crci: ADM DMA CRCI for data flow control
> + * @list:DMA descriptor list (list of desc_infos)
> + * @dma_done:completion param to denote end of last
> + * descriptor in the list
> + * @data_buffer: our local DMA buffer for page read/writes,
> + * used when we can't use the buffer provided
> + * by upper layers directly
> + * @buf_size/count/start:markers for chip->read_buf/write_buf functions
> + * @reg_read_buf:buffer for reading register data via DMA
> + * @reg_read_pos:marker for data read in reg_read_buf
> + * @cfg0, cfg1, cfg0_raw..: NANDc register configurations needed for
> + * ecc/non-ecc mode for the current nand flash
> + * device
> + * @regs:a contiguous chunk of memory for DMA register
> + * writes
> + * @ecc_strength:4 bit or 8 bit ecc, received via DT
> + * @bus_width: 8 bit or 16 bit NAND bus width,
> received via DT
> + * @ecc_modes: supported ECC modes by the current
> controller,
> + * initialized via DT match data
> + * @cw_size: the number of bytes in a single step/codeword
> + * of a page, consisting of all data, ecc, spare
> + * and reserved bytes
> + * @cw_data: the number of bytes within a codeword protected
> + *
Re: [PATCH v2 2/5] mtd: nand: Qualcomm NAND controller driver
On Tue, Jul 21, 2015 at 04:04:43PM +0530, Archit Taneja wrote: > > Signed-off-by: Archit Taneja > --- Reviewed-by: Andy Gross -- Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/5] mtd: nand: Qualcomm NAND controller driver
On Tue, Jul 21, 2015 at 04:04:43PM +0530, Archit Taneja wrote: snip Signed-off-by: Archit Taneja arch...@codeaurora.org --- Reviewed-by: Andy Gross agr...@codeaurora.org -- Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/5] mtd: nand: Qualcomm NAND controller driver
On 07/21/2015 03:34 AM, Archit Taneja wrote:
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 5b2806a..31951fc 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -538,4 +538,11 @@ config MTD_NAND_HISI504
help
Enables support for NAND controller on Hisilicon SoC Hip04.
+config MTD_NAND_QCOM
+ tristate Support for NAND on QCOM SoCs
+ depends on ARCH_QCOM QCOM_ADM
This is sort of annoying that the menu won't show up unless the ADM
driver is also enabled (which would be in a completely different area of
the configurator). Perhaps drop that requirement because it isn't
required to build?
+ help
+ Enables support for NAND flash chips on SoCs containing the EBI2 NAND
+ controller. This controller is found on IPQ806x SoC.
+
endif # MTD_NAND
diff --git a/drivers/mtd/nand/qcom_nandc.c b/drivers/mtd/nand/qcom_nandc.c
new file mode 100644
index 000..51c284c
--- /dev/null
+++ b/drivers/mtd/nand/qcom_nandc.c
@@ -0,0 +1,2019 @@
+/*
+ * Copyright (c) 2015, The Linux Foundation. All rights reserved.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include linux/clk.h
+#include linux/slab.h
+#include linux/interrupt.h
Where is this used?
+#include linux/bitops.h
+#include linux/dma-mapping.h
+#include linux/dmaengine.h
+#include linux/module.h
+#include linux/mtd/nand.h
+#include linux/mtd/partitions.h
+#include linux/of.h
+#include linux/of_device.h
+#include linux/of_mtd.h
+#include linux/delay.h
+
[..]
+/*
+ * the NAND controller performs reads/writes with ECC in 516 byte chunks.
+ * the driver calls the chunks 'step' or 'codeword' interchangeably
+ */
+#define NANDC_STEP_SIZE 512
+
+/*
+ * the largest page size we support is 8K, this will have 16 steps/codewords
+ * of 512 bytes each
+ */
+#define MAX_NUM_STEPS (SZ_8K / NANDC_STEP_SIZE)
+
+/* we read at most 3 registers per codeword scan */
+#define MAX_REG_RD (3 * MAX_NUM_STEPS)
+
+/* ECC modes */
+#define ECC_NONE BIT(0)
+#define ECC_RS_4BIT BIT(1)
+#define ECC_BCH_4BITBIT(2)
+#define ECC_BCH_8BITBIT(3)
+
+struct desc_info {
+ struct list_head list;
+
+ enum dma_transfer_direction dir;
+ struct scatterlist sgl;
+ struct dma_async_tx_descriptor *dma_desc;
+};
+
+/*
+ * holds the current register values that we want to write. acts as a
contiguous
+ * chunk of memory which we use to write the controller registers through
DMA.
+ */
+struct nandc_regs {
+ u32 cmd;
+ u32 addr0;
+ u32 addr1;
+ u32 chip_sel;
+ u32 exec;
+
+ u32 cfg0;
+ u32 cfg1;
+ u32 ecc_bch_cfg;
+
+ u32 clrflashstatus;
+ u32 clrreadstatus;
+
+ u32 cmd1;
+ u32 vld;
+
+ u32 orig_cmd1;
+ u32 orig_vld;
+
+ u32 ecc_buf_cfg;
+};
+
+/*
+ * @cmd_crci:ADM DMA CRCI for command flow control
+ * @data_crci: ADM DMA CRCI for data flow control
+ * @list:DMA descriptor list (list of desc_infos)
+ * @dma_done:completion param to denote end of last
+ * descriptor in the list
+ * @data_buffer: our local DMA buffer for page read/writes,
+ * used when we can't use the buffer provided
+ * by upper layers directly
+ * @buf_size/count/start:markers for chip-read_buf/write_buf functions
+ * @reg_read_buf:buffer for reading register data via DMA
+ * @reg_read_pos:marker for data read in reg_read_buf
+ * @cfg0, cfg1, cfg0_raw..: NANDc register configurations needed for
+ * ecc/non-ecc mode for the current nand flash
+ * device
+ * @regs:a contiguous chunk of memory for DMA register
+ * writes
+ * @ecc_strength:4 bit or 8 bit ecc, received via DT
+ * @bus_width: 8 bit or 16 bit NAND bus width,
received via DT
+ * @ecc_modes: supported ECC modes by the current
controller,
+ * initialized via DT match data
+ * @cw_size: the number of bytes in a single step/codeword
+ * of a page, consisting of all data, ecc, spare
+ * and reserved bytes
+ * @cw_data: the number of
[PATCH v2 2/5] mtd: nand: Qualcomm NAND controller driver
The Qualcomm NAND controller is found in SoCs like IPQ806x, MSM7xx, MDM9x15 series. It exists as a sub block inside the IPs EBI2 (External Bus Interface 2) and QPIC (Qualcomm Parallel Interface Controller). These IPs provide a broader interface for external slow peripheral devices such as LCD and NAND/NOR flash memory or SRAM like interfaces. We add support for the NAND controller found within EBI2. For the SoCs of our interest, we only use the NAND controller within EBI2. Therefore, it's safe for us to assume that the NAND controller is a standalone block within the SoC. The controller supports 512B, 2kB, 4kB and 8kB page 8-bit and 16-bit NAND flash devices. It contains a HW ECC block that supports BCH ECC (4, 8 and 16 bit correction/step) and RS ECC(4 bit correction/step) that covers main and spare data. The controller contains an internal 512 byte page buffer to which we read/write via DMA. The EBI2 type NAND controller uses ADM DMA for register read/write and data transfers. The controller performs page reads and writes at a codeword/step level of 512 bytes. It can support up to 2 external chips of different configurations. The driver prepares register read and write configuration descriptors for each codeword, followed by data descriptors to read or write data from the controller's internal buffer. It uses a single ADM DMA channel that we get via dmaengine API. The controller requires 2 ADM CRCIs for command and data flow control. These are passed via DT. The ecc layout used by the controller is syndrome like, but we can't use the standard syndrome ecc ops because of several reasons. First, the amount of data bytes covered by ecc isn't same in each step. Second, writing to free oob space requires us writing to the entire step in which the oob lies. This forces us to create our own ecc ops. One more difference is how the controller accesses the bad block marker. The controller ignores reading the marker when ECC is enabled. ECC needs to be explicity disabled to read or write to the bad block marker. For this reason, we use the newly created flag NAND_BBT_ACCESS_BBM_RAW to read the factory provided bad block markers. Signed-off-by: Archit Taneja --- drivers/mtd/nand/Kconfig |7 + drivers/mtd/nand/Makefile |1 + drivers/mtd/nand/qcom_nandc.c | 2019 + 3 files changed, 2027 insertions(+) create mode 100644 drivers/mtd/nand/qcom_nandc.c diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig index 5b2806a..31951fc 100644 --- a/drivers/mtd/nand/Kconfig +++ b/drivers/mtd/nand/Kconfig @@ -538,4 +538,11 @@ config MTD_NAND_HISI504 help Enables support for NAND controller on Hisilicon SoC Hip04. +config MTD_NAND_QCOM + tristate "Support for NAND on QCOM SoCs" + depends on ARCH_QCOM && QCOM_ADM + help + Enables support for NAND flash chips on SoCs containing the EBI2 NAND + controller. This controller is found on IPQ806x SoC. + endif # MTD_NAND diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 1f897ec..87b6a1d 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -53,5 +53,6 @@ obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH) += bcm47xxnflash/ obj-$(CONFIG_MTD_NAND_SUNXI) += sunxi_nand.o obj-$(CONFIG_MTD_NAND_HISI504) += hisi504_nand.o obj-$(CONFIG_MTD_NAND_BRCMNAND)+= brcmnand/ +obj-$(CONFIG_MTD_NAND_QCOM)+= qcom_nandc.o nand-objs := nand_base.o nand_bbt.o nand_timings.o diff --git a/drivers/mtd/nand/qcom_nandc.c b/drivers/mtd/nand/qcom_nandc.c new file mode 100644 index 000..51c284c --- /dev/null +++ b/drivers/mtd/nand/qcom_nandc.c @@ -0,0 +1,2019 @@ +/* + * Copyright (c) 2015, The Linux Foundation. All rights reserved. + * + * This software is licensed under the terms of the GNU General Public + * License version 2, as published by the Free Software Foundation, and + * may be copied, distributed, and modified under those terms. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/* NANDc reg offsets */ +#define NAND_FLASH_CMD 0x00 +#define NAND_ADDR0 0x04 +#define NAND_ADDR1 0x08 +#define NAND_FLASH_CHIP_SELECT 0x0c +#define NAND_EXEC_CMD 0x10 +#define NAND_FLASH_STATUS 0x14 +#define NAND_BUFFER_STATUS 0x18 +#define NAND_DEV0_CFG0 0x20 +#define NAND_DEV0_CFG1 0x24 +#define NAND_DEV0_ECC_CFG 0x28 +#define NAND_DEV1_ECC_CFG 0x2c +#define NAND_DEV1_CFG0 0x30 +#define
[PATCH v2 2/5] mtd: nand: Qualcomm NAND controller driver
The Qualcomm NAND controller is found in SoCs like IPQ806x, MSM7xx, MDM9x15 series. It exists as a sub block inside the IPs EBI2 (External Bus Interface 2) and QPIC (Qualcomm Parallel Interface Controller). These IPs provide a broader interface for external slow peripheral devices such as LCD and NAND/NOR flash memory or SRAM like interfaces. We add support for the NAND controller found within EBI2. For the SoCs of our interest, we only use the NAND controller within EBI2. Therefore, it's safe for us to assume that the NAND controller is a standalone block within the SoC. The controller supports 512B, 2kB, 4kB and 8kB page 8-bit and 16-bit NAND flash devices. It contains a HW ECC block that supports BCH ECC (4, 8 and 16 bit correction/step) and RS ECC(4 bit correction/step) that covers main and spare data. The controller contains an internal 512 byte page buffer to which we read/write via DMA. The EBI2 type NAND controller uses ADM DMA for register read/write and data transfers. The controller performs page reads and writes at a codeword/step level of 512 bytes. It can support up to 2 external chips of different configurations. The driver prepares register read and write configuration descriptors for each codeword, followed by data descriptors to read or write data from the controller's internal buffer. It uses a single ADM DMA channel that we get via dmaengine API. The controller requires 2 ADM CRCIs for command and data flow control. These are passed via DT. The ecc layout used by the controller is syndrome like, but we can't use the standard syndrome ecc ops because of several reasons. First, the amount of data bytes covered by ecc isn't same in each step. Second, writing to free oob space requires us writing to the entire step in which the oob lies. This forces us to create our own ecc ops. One more difference is how the controller accesses the bad block marker. The controller ignores reading the marker when ECC is enabled. ECC needs to be explicity disabled to read or write to the bad block marker. For this reason, we use the newly created flag NAND_BBT_ACCESS_BBM_RAW to read the factory provided bad block markers. Signed-off-by: Archit Taneja arch...@codeaurora.org --- drivers/mtd/nand/Kconfig |7 + drivers/mtd/nand/Makefile |1 + drivers/mtd/nand/qcom_nandc.c | 2019 + 3 files changed, 2027 insertions(+) create mode 100644 drivers/mtd/nand/qcom_nandc.c diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig index 5b2806a..31951fc 100644 --- a/drivers/mtd/nand/Kconfig +++ b/drivers/mtd/nand/Kconfig @@ -538,4 +538,11 @@ config MTD_NAND_HISI504 help Enables support for NAND controller on Hisilicon SoC Hip04. +config MTD_NAND_QCOM + tristate Support for NAND on QCOM SoCs + depends on ARCH_QCOM QCOM_ADM + help + Enables support for NAND flash chips on SoCs containing the EBI2 NAND + controller. This controller is found on IPQ806x SoC. + endif # MTD_NAND diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 1f897ec..87b6a1d 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -53,5 +53,6 @@ obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH) += bcm47xxnflash/ obj-$(CONFIG_MTD_NAND_SUNXI) += sunxi_nand.o obj-$(CONFIG_MTD_NAND_HISI504) += hisi504_nand.o obj-$(CONFIG_MTD_NAND_BRCMNAND)+= brcmnand/ +obj-$(CONFIG_MTD_NAND_QCOM)+= qcom_nandc.o nand-objs := nand_base.o nand_bbt.o nand_timings.o diff --git a/drivers/mtd/nand/qcom_nandc.c b/drivers/mtd/nand/qcom_nandc.c new file mode 100644 index 000..51c284c --- /dev/null +++ b/drivers/mtd/nand/qcom_nandc.c @@ -0,0 +1,2019 @@ +/* + * Copyright (c) 2015, The Linux Foundation. All rights reserved. + * + * This software is licensed under the terms of the GNU General Public + * License version 2, as published by the Free Software Foundation, and + * may be copied, distributed, and modified under those terms. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include linux/clk.h +#include linux/slab.h +#include linux/interrupt.h +#include linux/bitops.h +#include linux/dma-mapping.h +#include linux/dmaengine.h +#include linux/module.h +#include linux/mtd/nand.h +#include linux/mtd/partitions.h +#include linux/of.h +#include linux/of_device.h +#include linux/of_mtd.h +#include linux/delay.h + +/* NANDc reg offsets */ +#define NAND_FLASH_CMD 0x00 +#define NAND_ADDR0 0x04 +#define NAND_ADDR1 0x08 +#define NAND_FLASH_CHIP_SELECT 0x0c +#define NAND_EXEC_CMD 0x10 +#define NAND_FLASH_STATUS 0x14 +#define NAND_BUFFER_STATUS 0x18 +#define NAND_DEV0_CFG0
