| Message ID | 20170905202339.20732-1-wak@google.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Boris Brezillon |
| Headers | show |
| Series | mtd: powernv: Support MTD_NO_ERASE | expand |
+the DT maintainers On Tue, 5 Sep 2017 13:23:39 -0700 "William A. Kennington III" <wak@google.com> wrote: > As of skiboot@ba99af9b149d02438347b055e6e7d6bd15e33551, we now support I didn't find this commit in mainline. > 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 <wak@google.com> > --- > drivers/mtd/devices/powernv_flash.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/mtd/devices/powernv_flash.c b/drivers/mtd/devices/powernv_flash.c > index f5396f26ddb4..fab9f9121b4c 100644 > --- a/drivers/mtd/devices/powernv_flash.c > +++ b/drivers/mtd/devices/powernv_flash.c > @@ -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? > mtd->size = size; > mtd->erasesize = erase_size; > mtd->writebufsize = mtd->writesize = 1;
William Kennington <wak@google.com> writes: > +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? > 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 > > On Thu, Sep 21, 2017 at 12:59 AM Boris Brezillon < > boris.brezillon@free-electrons.com> wrote: > >> +the DT maintainers >> >> On Tue, 5 Sep 2017 13:23:39 -0700 >> "William A. Kennington III" <wak@google.com> wrote: >> >> > As of skiboot@ba99af9b149d02438347b055e6e7d6bd15e33551, we now support >> >> I didn't find this commit in mainline. >> >> > 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 <wak@google.com> >> > --- >> > drivers/mtd/devices/powernv_flash.c | 2 ++ >> > 1 file changed, 2 insertions(+) >> > >> > diff --git a/drivers/mtd/devices/powernv_flash.c >> b/drivers/mtd/devices/powernv_flash.c >> > index f5396f26ddb4..fab9f9121b4c 100644 >> > --- a/drivers/mtd/devices/powernv_flash.c >> > +++ b/drivers/mtd/devices/powernv_flash.c >> > @@ -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? 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
Hi Michael, Sorry for the late reply. On Fri, 22 Sep 2017 14:13:47 +1000 Michael Ellerman <mpe@ellerman.id.au> wrote: > William Kennington <wak@google.com> writes: > > > +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? > > > 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 > > > > On Thu, Sep 21, 2017 at 12:59 AM Boris Brezillon < > > boris.brezillon@free-electrons.com> wrote: > > > >> +the DT maintainers > >> > >> On Tue, 5 Sep 2017 13:23:39 -0700 > >> "William A. Kennington III" <wak@google.com> wrote: > >> > >> > As of skiboot@ba99af9b149d02438347b055e6e7d6bd15e33551, we now support > >> > >> I didn't find this commit in mainline. > >> > >> > 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 <wak@google.com> > >> > --- > >> > drivers/mtd/devices/powernv_flash.c | 2 ++ > >> > 1 file changed, 2 insertions(+) > >> > > >> > diff --git a/drivers/mtd/devices/powernv_flash.c > >> b/drivers/mtd/devices/powernv_flash.c > >> > index f5396f26ddb4..fab9f9121b4c 100644 > >> > --- a/drivers/mtd/devices/powernv_flash.c > >> > +++ b/drivers/mtd/devices/powernv_flash.c > >> > @@ -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? > > 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 :-). > > 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?
On Mon, Oct 16, 2017 at 10:55 AM, Boris Brezillon <boris.brezillon@free-electrons.com> wrote: > Hi Michael, > > Sorry for the late reply. > > On Fri, 22 Sep 2017 14:13:47 +1000 > Michael Ellerman <mpe@ellerman.id.au> wrote: > >> William Kennington <wak@google.com> writes: >> >> > +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? >> >> > 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 >> > >> > On Thu, Sep 21, 2017 at 12:59 AM Boris Brezillon < >> > boris.brezillon@free-electrons.com> wrote: >> > >> >> +the DT maintainers >> >> >> >> On Tue, 5 Sep 2017 13:23:39 -0700 >> >> "William A. Kennington III" <wak@google.com> wrote: >> >> >> >> > As of skiboot@ba99af9b149d02438347b055e6e7d6bd15e33551, we now support >> >> >> >> I didn't find this commit in mainline. >> >> >> >> > 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 <wak@google.com> >> >> > --- >> >> > drivers/mtd/devices/powernv_flash.c | 2 ++ >> >> > 1 file changed, 2 insertions(+) >> >> > >> >> > diff --git a/drivers/mtd/devices/powernv_flash.c >> >> b/drivers/mtd/devices/powernv_flash.c >> >> > index f5396f26ddb4..fab9f9121b4c 100644 >> >> > --- a/drivers/mtd/devices/powernv_flash.c >> >> > +++ b/drivers/mtd/devices/powernv_flash.c >> >> > @@ -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? >> >> 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 :-). > >> >> 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. > 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
Rob Herring <robh+dt@kernel.org> writes: > 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)
On Wed, 18 Oct 2017 16:11:33 +1100 Stewart Smith <stewart@linux.vnet.ibm.com> wrote: > Rob Herring <robh+dt@kernel.org> writes: > > 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? > > 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) >
On Wed, Oct 18, 2017 at 12:11 AM, Stewart Smith <stewart@linux.vnet.ibm.com> wrote: > Rob Herring <robh+dt@kernel.org> writes: >> 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. > 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
diff --git a/drivers/mtd/devices/powernv_flash.c b/drivers/mtd/devices/powernv_flash.c index f5396f26ddb4..fab9f9121b4c 100644 --- a/drivers/mtd/devices/powernv_flash.c +++ b/drivers/mtd/devices/powernv_flash.c @@ -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;
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 <wak@google.com> --- drivers/mtd/devices/powernv_flash.c | 2 ++ 1 file changed, 2 insertions(+)