[PATCH] mtd: powernv: Support MTD_NO_ERASE

[PATCH] mtd: powernv: Support MTD_NO_ERASE

[William A. Kennington III]

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(+)

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;
-- 
2.13.1

Re: [PATCH] mtd: powernv: Support MTD_NO_ERASE

[Boris Brezillon]

+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;

Re: [PATCH] mtd: powernv: Support MTD_NO_ERASE

[Michael Ellerman]

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

Re: [PATCH] mtd: powernv: Support MTD_NO_ERASE

[Boris Brezillon]

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?

Re: [PATCH] mtd: powernv: Support MTD_NO_ERASE

[Rob Herring]

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

Re: [PATCH] mtd: powernv: Support MTD_NO_ERASE

[Stewart Smith]

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)

-- 
Stewart Smith
OPAL Architect, IBM.

Re: [PATCH] mtd: powernv: Support MTD_NO_ERASE

[Boris Brezillon]

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)
> 

Re: [PATCH] mtd: powernv: Support MTD_NO_ERASE

[Rob Herring]

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