mtd: powernv: Support MTD_NO_ERASE

PATCH In review William A. Kennington III 5 participants 9 messages Sep 5, 2017 – Oct 30, 2017 5 messages not in the archive

Subsystems (2)

PowerPC Memory technology devices (MTD)

lore

Share & tools

lore

William A. Kennington III

author

2017-09-05 20:24 UTC #0 lore raw

PATCH 1/1 mtd: powernv: Support MTD_NO_ERASE

As of skiboot@ba99af9b149d02438347b055e6e7d6bd15e33551, we now support adding a device tree entry which marks a flash device as not needing erase. This patch adds support for setting MTD_NO_ERASE on powernvflash devices which have the property no-erase.

Signed-off-by William A. Kennington III <[email protected]>

---
 drivers/mtd/devices/powernv_flash.c | 2 ++
 1 file changed, 2 insertions(+)

drivers/mtd/devices/powernv_flash.c +2 -0

index f5396f26ddb4..fab9f9121b4c 100644
@@ -196,6 +196,8 @@ static int powernv_flash_set_driver_info(struct device *dev,
 	mtd->name = of_get_property(dev->of_node, "name", NULL);
 	mtd->type = MTD_NORFLASH;
 	mtd->flags = MTD_WRITEABLE;
+	if (of_property_read_bool(dev->of_node, "no-erase"))
+		mtd->flags |= MTD_NO_ERASE;
 	mtd->size = size;
 	mtd->erasesize = erase_size;
 	mtd->writebufsize = mtd->writesize = 1;

Signature · 1 line

2.13.1

A message that is not in the archive (sent off-list or not yet received): 20170905202339.20732-1-wak-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org

Boris Brezillon

2017-09-21 07:59 UTC #2 lore raw

+the DT maintainers

Quoted 1 line · On Tue, 5 Sep 2017 13:23:39 -0700 "William A. Kennington III"…

As of skiboot@ba99af9b149d02438347b055e6e7d6bd15e33551, we now support

I didn't find this commit in mainline.

Quoted 14 lines

adding a device tree entry which marks a flash device as not needing erase. This patch adds support for setting MTD_NO_ERASE on powernvflash devices which have the property no-erase.
Signed-off-by William A. Kennington III <[email protected]>
---
 drivers/mtd/devices/powernv_flash.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/mtd/devices/powernv_flash.c +0 -0
index f5396f26ddb4..fab9f9121b4c 100644

drivers/mtd/devices/powernv_flash.c static int powernv_flash_set_driver_info(struct device *dev, · line 200

Quoted 5 lines

drivers/mtd/devices/powernv_flash.c +2 -0

@@ -196,6 +196,8 @@ static int powernv_flash_set_driver_info(struct device *dev,
 	mtd->name = of_get_property(dev->of_node, "name", NULL);
 	mtd->type = MTD_NORFLASH;
 	mtd->flags = MTD_WRITEABLE;
+	if (of_property_read_bool(dev->of_node, "no-erase"))
+		mtd->flags |= MTD_NO_ERASE;

I don't see this property document in a DT binding doc, which is mandatory. Actually, there's no DT bindings doc at all for the 'ibm,opal-flash' device in mainline, is this normal?

drivers/mtd/devices/powernv_flash.c static int powernv_flash_set_driver_info(struct device *dev, · line 203

Quoted 3 lines

drivers/mtd/devices/powernv_flash.c +0 -0

 	mtd->size = size;
 	mtd->erasesize = erase_size;
 	mtd->writebufsize = mtd->writesize = 1;

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

William Kennington

author

2017-09-21 19:15 UTC #3 lore raw

+Cyril Bur and

Not sure what you mean by mainline, but the commit is in the master branch at
https://github.com/open-power/skiboot/commit/ba99af9b149d02438347b055e6e7d6bd15e33551

I'm unsure about the documentation of the properties as I'm not really the maintainer of any of the code on either side. I just added this DT property for plumbing through the NO_ERASE flag. I added Cyril and Michael as they both work closely with this driver and could comment better than I could.

- William

Quoted 40 lines · Boris Brezillon

+the DT maintainers
Quoted 1 line · On Tue, 5 Sep 2017 13:23:39 -0700 "William A. Kennington III"…
As of skiboot@ba99af9b149d02438347b055e6e7d6bd15e33551, we now support
I didn't find this commit in mainline.
Review
Quoted 10 lines
adding a device tree entry which marks a flash device as not needing erase. This patch adds support for setting MTD_NO_ERASE on powernvflash devices which have the property no-erase.
Signed-off-by William A. Kennington III <[email protected]>
---
 drivers/mtd/devices/powernv_flash.c | 2 ++
 1 file changed, 2 insertions(+)
(quoted diff) +0 -0
b/drivers/mtd/devices/powernv_flash.c
drivers/mtd/devices/powernv_flash.c static int powernv_flash_set_driver_info(struct
Quoted 4 lines
index f5396f26ddb4..fab9f9121b4c 100644
drivers/mtd/devices/powernv_flash.c +0 -0
@@ -196,6 +196,8 @@ static int powernv_flash_set_driver_info(struct
device *dev,
drivers/mtd/devices/powernv_flash.c static int powernv_flash_set_driver_info(struct device *dev, · line 200
Quoted 5 lines
drivers/mtd/devices/powernv_flash.c +2 -0
      mtd->name = of_get_property(dev->of_node, "name", NULL);
      mtd->type = MTD_NORFLASH;
      mtd->flags = MTD_WRITEABLE;
+     if (of_property_read_bool(dev->of_node, "no-erase"))
+             mtd->flags |= MTD_NO_ERASE;
I don't see this property document in a DT binding doc, which is mandatory. Actually, there's no DT bindings doc at all for the 'ibm,opal-flash' device in mainline, is this normal?
Quoted 3 lines
drivers/mtd/devices/powernv_flash.c +0 -0
      mtd->size = size;
      mtd->erasesize = erase_size;
      mtd->writebufsize = mtd->writesize = 1;

A message that is not in the archive (sent off-list or not yet received): CAPnigKmBujCoKdtXwnK6YsU=YvXM+MaDvDvKpUXEVgHzOC-VEw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

Michael Ellerman

2017-09-22 04:13 UTC #5 lore raw

Quoted 5 lines · William Kennington

+Cyril Bur and
Not sure what you mean by mainline, but the commit is in the master branch at
https://github.com/open-power/skiboot/commit/ba99af9b149d02438347b055e6e7d6bd15e33551

I suspect Boris missed that you were referring to a skiboot commit and looked in Linux?

Quoted 5 lines

I'm unsure about the documentation of the properties as I'm not really the maintainer of any of the code on either side. I just added this DT property for plumbing through the NO_ERASE flag. I added Cyril and Michael as they both work closely with this driver and could comment better than I could.

Quoted 39 lines

- William
Quoted 34 lines · Boris Brezillon
+the DT maintainers
Quoted 1 line · On Tue, 5 Sep 2017 13:23:39 -0700 "William A. Kennington III"…
As of skiboot@ba99af9b149d02438347b055e6e7d6bd15e33551, we now support
I didn't find this commit in mainline.
Review
Quoted 10 lines
adding a device tree entry which marks a flash device as not needing erase. This patch adds support for setting MTD_NO_ERASE on powernvflash devices which have the property no-erase.
Signed-off-by William A. Kennington III <[email protected]>
---
 drivers/mtd/devices/powernv_flash.c | 2 ++
 1 file changed, 2 insertions(+)
(quoted diff) +0 -0
b/drivers/mtd/devices/powernv_flash.c
drivers/mtd/devices/powernv_flash.c static int powernv_flash_set_driver_info(struct
Quoted 4 lines
index f5396f26ddb4..fab9f9121b4c 100644
drivers/mtd/devices/powernv_flash.c +0 -0
@@ -196,6 +196,8 @@ static int powernv_flash_set_driver_info(struct
device *dev,
drivers/mtd/devices/powernv_flash.c static int powernv_flash_set_driver_info(struct device *dev, · line 200
Quoted 5 lines
drivers/mtd/devices/powernv_flash.c +2 -0
      mtd->name = of_get_property(dev->of_node, "name", NULL);
      mtd->type = MTD_NORFLASH;
      mtd->flags = MTD_WRITEABLE;
+     if (of_property_read_bool(dev->of_node, "no-erase"))
+             mtd->flags |= MTD_NO_ERASE;
I don't see this property document in a DT binding doc, which is mandatory. Actually, there's no DT bindings doc at all for the 'ibm,opal-flash' device in mainline, is this normal?

The binding doc is here:

https://github.com/open-power/skiboot/blob/master/doc/device-tree/ibm%2Copal/flash.rst

But that property is not documented :(

We have some of the skiboot bindings also in the kernel tree, but there was talk of moving all the binding docs *out* of the kernel, so we never made an effort to put more in. I've lost track of whether there are still plans to move the binding docs out or not.

cheers
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 8760cbv090.fsf-W0DJWXSxmBNbyGPkN3NxC2scP1bn1w/[email protected]

Boris Brezillon

2017-10-16 15:55 UTC #7 lore raw

Hi Michael,

Sorry for the late reply.

Quoted 56 lines · On Fri, 22 Sep 2017 14:13:47 +1000 Michael Ellerman…

Quoted 5 lines · William Kennington
+Cyril Bur and
Not sure what you mean by mainline, but the commit is in the master branch at
https://github.com/open-power/skiboot/commit/ba99af9b149d02438347b055e6e7d6bd15e33551
I suspect Boris missed that you were referring to a skiboot commit and looked in Linux?
Quoted 5 lines
I'm unsure about the documentation of the properties as I'm not really the maintainer of any of the code on either side. I just added this DT property for plumbing through the NO_ERASE flag. I added Cyril and Michael as they both work closely with this driver and could comment better than I could.
Quoted 39 lines
- William
Quoted 34 lines · Boris Brezillon
+the DT maintainers
Quoted 1 line · On Tue, 5 Sep 2017 13:23:39 -0700 "William A. Kennington III"…
As of skiboot@ba99af9b149d02438347b055e6e7d6bd15e33551, we now support
I didn't find this commit in mainline.
Review
Quoted 10 lines
adding a device tree entry which marks a flash device as not needing erase. This patch adds support for setting MTD_NO_ERASE on powernvflash devices which have the property no-erase.
Signed-off-by William A. Kennington III <[email protected]>
---
 drivers/mtd/devices/powernv_flash.c | 2 ++
 1 file changed, 2 insertions(+)
(quoted diff) +0 -0
b/drivers/mtd/devices/powernv_flash.c
drivers/mtd/devices/powernv_flash.c static int powernv_flash_set_driver_info(struct
Quoted 4 lines
index f5396f26ddb4..fab9f9121b4c 100644
drivers/mtd/devices/powernv_flash.c +0 -0
@@ -196,6 +196,8 @@ static int powernv_flash_set_driver_info(struct  
device *dev,
drivers/mtd/devices/powernv_flash.c static int powernv_flash_set_driver_info(struct · line 200
Quoted 5 lines
drivers/mtd/devices/powernv_flash.c +2 -0
      mtd->name = of_get_property(dev->of_node, "name", NULL);
      mtd->type = MTD_NORFLASH;
      mtd->flags = MTD_WRITEABLE;
+     if (of_property_read_bool(dev->of_node, "no-erase"))
+             mtd->flags |= MTD_NO_ERASE;  
I don't see this property document in a DT binding doc, which is mandatory. Actually, there's no DT bindings doc at all for the 'ibm,opal-flash' device in mainline, is this normal?

Quoted 5 lines

The binding doc is here:
https://github.com/open-power/skiboot/blob/master/doc/device-tree/ibm%2Copal/flash.rst
But that property is not documented :(

Would be good to update the doc then :-).

Quoted 5 lines

We have some of the skiboot bindings also in the kernel tree, but there was talk of moving all the binding docs *out* of the kernel, so we never made an effort to put more in. I've lost track of whether there are still plans to move the binding docs out or not.

Okay, I don't know what's the policy for external DT bindings doc, but maybe you can add a file in Document/devicetree/bindings/mtd pointing to the skiboot doc.

Rob, what's your opinion?
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Rob Herring

2017-10-17 13:21 UTC #8 lore raw

Quoted 63 lines · Boris Brezillon

Hi Michael,
Sorry for the late reply.
Quoted 56 lines · On Fri, 22 Sep 2017 14:13:47 +1000 Michael Ellerman…
Quoted 5 lines · William Kennington
+Cyril Bur and
Not sure what you mean by mainline, but the commit is in the master branch at
https://github.com/open-power/skiboot/commit/ba99af9b149d02438347b055e6e7d6bd15e33551
I suspect Boris missed that you were referring to a skiboot commit and looked in Linux?
Quoted 44 lines
I'm unsure about the documentation of the properties as I'm not really the maintainer of any of the code on either side. I just added this DT property for plumbing through the NO_ERASE flag. I added Cyril and Michael as they both work closely with this driver and could comment better than I could.
- William
Quoted 34 lines · Boris Brezillon
+the DT maintainers
Quoted 1 line · On Tue, 5 Sep 2017 13:23:39 -0700 "William A. Kennington III"…
As of skiboot@ba99af9b149d02438347b055e6e7d6bd15e33551, we now support
I didn't find this commit in mainline.
Review
Quoted 10 lines
adding a device tree entry which marks a flash device as not needing erase. This patch adds support for setting MTD_NO_ERASE on powernvflash devices which have the property no-erase.
Signed-off-by William A. Kennington III <[email protected]>
---
 drivers/mtd/devices/powernv_flash.c | 2 ++
 1 file changed, 2 insertions(+)
(quoted diff) +0 -0
b/drivers/mtd/devices/powernv_flash.c
drivers/mtd/devices/powernv_flash.c static int powernv_flash_set_driver_info(struct
Quoted 4 lines
index f5396f26ddb4..fab9f9121b4c 100644
drivers/mtd/devices/powernv_flash.c +0 -0
@@ -196,6 +196,8 @@ static int powernv_flash_set_driver_info(struct
device *dev,
drivers/mtd/devices/powernv_flash.c static int powernv_flash_set_driver_info(struct · line 200
Quoted 5 lines
drivers/mtd/devices/powernv_flash.c +2 -0
      mtd->name = of_get_property(dev->of_node, "name", NULL);
      mtd->type = MTD_NORFLASH;
      mtd->flags = MTD_WRITEABLE;
+     if (of_property_read_bool(dev->of_node, "no-erase"))
+             mtd->flags |= MTD_NO_ERASE;
I don't see this property document in a DT binding doc, which is mandatory. Actually, there's no DT bindings doc at all for the 'ibm,opal-flash' device in mainline, is this normal?

Quoted 13 lines

Quoted 5 lines
The binding doc is here:
https://github.com/open-power/skiboot/blob/master/doc/device-tree/ibm%2Copal/flash.rst
But that property is not documented :(
Would be good to update the doc then :-).
Quoted 5 lines
We have some of the skiboot bindings also in the kernel tree, but there was talk of moving all the binding docs *out* of the kernel, so we never made an effort to put more in. I've lost track of whether there are still plans to move the binding docs out or not.

Whether bindings are moved out or not, what's in the kernel will be the starting point. I wouldn't hold your breath for them to move out any time soon.

Quoted 5 lines

Okay, I don't know what's the policy for external DT bindings doc, but maybe you can add a file in Document/devicetree/bindings/mtd pointing to the skiboot doc.
Rob, what's your opinion?

I'd prefer they be documented in the kernel especially if they are used by the kernel. Of course, there's IBM bindings covered by various ?PAPR specs which wouldn't make sense to duplicate, but I'm guessing what's documented in skiboot are not covered by those specs.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): CAL_JsqLBkU9habdOhoQ8ZiM=x87eLmuLrS6ujx32UOv8LHc=EA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

Stewart Smith

2017-10-18 05:11 UTC #10 lore raw

Quoted 4 lines · Rob Herring

I'd prefer they be documented in the kernel especially if they are used by the kernel. Of course, there's IBM bindings covered by various ?PAPR specs which wouldn't make sense to duplicate, but I'm guessing what's documented in skiboot are not covered by those specs.

Correct.

I don't have an objection to docs being in both places, as long as we could keep both in sync. Would suitable comments in the RST doc files pointing to the skiboot repo as the canonical copy work?

I'd like to be able to just diff a subtree of kernel and skiboot to see if the docs have diverged.

We may also need to think of a way to ensure that any of that moving back/forth of edits is done in a way that makes licensing sense too (skiboot is Apache 2.0)

Signature · 7 lines

Stewart Smith
OPAL Architect, IBM.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): [email protected]

Boris Brezillon

2017-10-30 08:48 UTC #12 lore raw

Quoted 11 lines · On Wed, 18 Oct 2017 16:11:33 +1100 Stewart Smith…

Quoted 4 lines · Rob Herring
I'd prefer they be documented in the kernel especially if they are used by the kernel. Of course, there's IBM bindings covered by various ?PAPR specs which wouldn't make sense to duplicate, but I'm guessing what's documented in skiboot are not covered by those specs.
Correct.
I don't have an objection to docs being in both places, as long as we could keep both in sync. Would suitable comments in the RST doc files pointing to the skiboot repo as the canonical copy work?

Rob, any opinion on that?

Quoted 8 lines

I'd like to be able to just diff a subtree of kernel and skiboot to see if the docs have diverged.
We may also need to think of a way to ensure that any of that moving back/forth of edits is done in a way that makes licensing sense too (skiboot is Apache 2.0)

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Rob Herring

2017-10-30 15:23 UTC #13 lore raw

In reply to a message not in the archive

Quoted 11 lines · Stewart Smith

Quoted 4 lines · Rob Herring
I'd prefer they be documented in the kernel especially if they are used by the kernel. Of course, there's IBM bindings covered by various ?PAPR specs which wouldn't make sense to duplicate, but I'm guessing what's documented in skiboot are not covered by those specs.
Correct.
I don't have an objection to docs being in both places, as long as we could keep both in sync. Would suitable comments in the RST doc files pointing to the skiboot repo as the canonical copy work?

I'm okay if the kernel just references the skiboot doc and when we review that, we can review the link. If it's something intended to be common (among devices), then I'd like it to be documented within the main binding docs (i.e. the kernel ones).

Of course, I will feel differently if everyone wanted to do this or the bindings conflict with upstream bindings.

Quoted 6 lines

I'd like to be able to just diff a subtree of kernel and skiboot to see if the docs have diverged.
We may also need to think of a way to ensure that any of that moving back/forth of edits is done in a way that makes licensing sense too (skiboot is Apache 2.0)

Yeah, that's not the only licensing issue. Any volunteers to clean up ambiguous licensing.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html