mbox series

[v2,0/3] mtd: rawnand: denali: add new clocks and improve setup_data_interface

Message ID 1528953067-24324-1-git-send-email-yamada.masahiro@socionext.com
Headers show
Series mtd: rawnand: denali: add new clocks and improve setup_data_interface | expand

Message

Masahiro Yamada June 14, 2018, 5:11 a.m. UTC
The ->setup_data_interface() hook needs to know the clock frequency.
In fact, this IP needs three clocks, bus "which clock?" is really
confusing.  (It is not described in the DT-binding at all.)

This series adds more clocks.  In the new binding, three clocks
are required: core clock, bus interface clock, ECC engine clock.

This series also takes care of the backward compatibility by
providing hardcoded values in case the new clocks are missing.
So, existing DT should work.


Changes in v2:
  - Split patches into sensible chunks

Masahiro Yamada (3):
  mtd: rawnand: denali_dt: use dev as a shorthand of &pdev->dev
  mtd: rawnand: denali_dt: add more clocks based on IP datasheet
  mtd: rawnand: denali: optimize timing parameters for data interface

 .../devicetree/bindings/mtd/denali-nand.txt        |  5 ++
 drivers/mtd/nand/raw/denali.c                      | 49 ++++++++--------
 drivers/mtd/nand/raw/denali.h                      |  1 +
 drivers/mtd/nand/raw/denali_dt.c                   | 66 ++++++++++++++++++----
 drivers/mtd/nand/raw/denali_pci.c                  |  1 +
 5 files changed, 86 insertions(+), 36 deletions(-)

Comments

Richard Weinberger June 14, 2018, 7:25 a.m. UTC | #1
Masahiro,

Am Donnerstag, 14. Juni 2018, 07:11:04 CEST schrieb Masahiro Yamada:
> 
> The ->setup_data_interface() hook needs to know the clock frequency.
> In fact, this IP needs three clocks, bus "which clock?" is really
> confusing.  (It is not described in the DT-binding at all.)
> 
> This series adds more clocks.  In the new binding, three clocks
> are required: core clock, bus interface clock, ECC engine clock.
> 
> This series also takes care of the backward compatibility by
> providing hardcoded values in case the new clocks are missing.
> So, existing DT should work.
> 
> 
> Changes in v2:
>   - Split patches into sensible chunks
> 
> Masahiro Yamada (3):
>   mtd: rawnand: denali_dt: use dev as a shorthand of &pdev->dev
>   mtd: rawnand: denali_dt: add more clocks based on IP datasheet
>   mtd: rawnand: denali: optimize timing parameters for data interface

This still means that we need to feed at least 2/3 and 3/3 into -stable to
unbreak the driver.
I'd really prefer a single self-hosting fix that can go into -stable and then
patches on top of the fix can prettify the driver.

Thanks,
//richard
Masahiro Yamada June 14, 2018, 7:29 a.m. UTC | #2
Hi Richard,

2018-06-14 16:25 GMT+09:00 Richard Weinberger <richard@sigma-star.at>:
> Masahiro,
>
> Am Donnerstag, 14. Juni 2018, 07:11:04 CEST schrieb Masahiro Yamada:
>>
>> The ->setup_data_interface() hook needs to know the clock frequency.
>> In fact, this IP needs three clocks, bus "which clock?" is really
>> confusing.  (It is not described in the DT-binding at all.)
>>
>> This series adds more clocks.  In the new binding, three clocks
>> are required: core clock, bus interface clock, ECC engine clock.
>>
>> This series also takes care of the backward compatibility by
>> providing hardcoded values in case the new clocks are missing.
>> So, existing DT should work.
>>
>>
>> Changes in v2:
>>   - Split patches into sensible chunks
>>
>> Masahiro Yamada (3):
>>   mtd: rawnand: denali_dt: use dev as a shorthand of &pdev->dev
>>   mtd: rawnand: denali_dt: add more clocks based on IP datasheet
>>   mtd: rawnand: denali: optimize timing parameters for data interface
>
> This still means that we need to feed at least 2/3 and 3/3 into -stable to
> unbreak the driver.


3/3 is not mandatory.

You can only back-port 1/3 and 2/3.


3/3 is an optimization.
It will make the device access a little faster,
but you do not have to take it if you do not want to.




> I'd really prefer a single self-hosting fix that can go into -stable and then
> patches on top of the fix can prettify the driver.
>
> Thanks,
> //richard
>
> --
> sigma star gmbh - Eduard-Bodem-Gasse 6 - 6020 Innsbruck - Austria
> ATU66964118 - FN 374287y
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Boris Brezillon June 14, 2018, 7:38 a.m. UTC | #3
On Thu, 14 Jun 2018 16:29:59 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:

> Hi Richard,
> 
> 2018-06-14 16:25 GMT+09:00 Richard Weinberger <richard@sigma-star.at>:
> > Masahiro,
> >
> > Am Donnerstag, 14. Juni 2018, 07:11:04 CEST schrieb Masahiro Yamada:  
> >>
> >> The ->setup_data_interface() hook needs to know the clock frequency.
> >> In fact, this IP needs three clocks, bus "which clock?" is really
> >> confusing.  (It is not described in the DT-binding at all.)
> >>
> >> This series adds more clocks.  In the new binding, three clocks
> >> are required: core clock, bus interface clock, ECC engine clock.
> >>
> >> This series also takes care of the backward compatibility by
> >> providing hardcoded values in case the new clocks are missing.
> >> So, existing DT should work.
> >>
> >>
> >> Changes in v2:
> >>   - Split patches into sensible chunks
> >>
> >> Masahiro Yamada (3):
> >>   mtd: rawnand: denali_dt: use dev as a shorthand of &pdev->dev
> >>   mtd: rawnand: denali_dt: add more clocks based on IP datasheet
> >>   mtd: rawnand: denali: optimize timing parameters for data interface  
> >
> > This still means that we need to feed at least 2/3 and 3/3 into -stable to
> > unbreak the driver.  
> 
> 
> 3/3 is not mandatory.
> 
> You can only back-port 1/3 and 2/3.

Well, patch 1 is not a fix, can we move it after patch 2 so that only
patch 2 is flagged with the Fixes+Cc-stable tags?

Thanks,

Boris

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Richard Weinberger June 14, 2018, 8:02 a.m. UTC | #4
Am Donnerstag, 14. Juni 2018, 09:38:35 CEST schrieb Boris Brezillon:
> On Thu, 14 Jun 2018 16:29:59 +0900
> Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
> 
> > Hi Richard,
> > 
> > 2018-06-14 16:25 GMT+09:00 Richard Weinberger <richard@sigma-star.at>:
> > > Masahiro,
> > >
> > > Am Donnerstag, 14. Juni 2018, 07:11:04 CEST schrieb Masahiro Yamada:  
> > >>
> > >> The ->setup_data_interface() hook needs to know the clock frequency.
> > >> In fact, this IP needs three clocks, bus "which clock?" is really
> > >> confusing.  (It is not described in the DT-binding at all.)
> > >>
> > >> This series adds more clocks.  In the new binding, three clocks
> > >> are required: core clock, bus interface clock, ECC engine clock.
> > >>
> > >> This series also takes care of the backward compatibility by
> > >> providing hardcoded values in case the new clocks are missing.
> > >> So, existing DT should work.
> > >>
> > >>
> > >> Changes in v2:
> > >>   - Split patches into sensible chunks
> > >>
> > >> Masahiro Yamada (3):
> > >>   mtd: rawnand: denali_dt: use dev as a shorthand of &pdev->dev
> > >>   mtd: rawnand: denali_dt: add more clocks based on IP datasheet
> > >>   mtd: rawnand: denali: optimize timing parameters for data interface  
> > >
> > > This still means that we need to feed at least 2/3 and 3/3 into -stable to
> > > unbreak the driver.  
> > 
> > 
> > 3/3 is not mandatory.
> > 
> > You can only back-port 1/3 and 2/3.
> 
> Well, patch 1 is not a fix, can we move it after patch 2 so that only
> patch 2 is flagged with the Fixes+Cc-stable tags?

Another question, shall we fix arch/arm/boot/dts/socfpga.dtsi too?

Thanks,
//richard
Boris Brezillon June 14, 2018, 8:03 a.m. UTC | #5
On Thu, 14 Jun 2018 10:02:12 +0200
Richard Weinberger <richard@sigma-star.at> wrote:

> Am Donnerstag, 14. Juni 2018, 09:38:35 CEST schrieb Boris Brezillon:
> > On Thu, 14 Jun 2018 16:29:59 +0900
> > Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
> >   
> > > Hi Richard,
> > > 
> > > 2018-06-14 16:25 GMT+09:00 Richard Weinberger <richard@sigma-star.at>:  
> > > > Masahiro,
> > > >
> > > > Am Donnerstag, 14. Juni 2018, 07:11:04 CEST schrieb Masahiro Yamada:    
> > > >>
> > > >> The ->setup_data_interface() hook needs to know the clock frequency.
> > > >> In fact, this IP needs three clocks, bus "which clock?" is really
> > > >> confusing.  (It is not described in the DT-binding at all.)
> > > >>
> > > >> This series adds more clocks.  In the new binding, three clocks
> > > >> are required: core clock, bus interface clock, ECC engine clock.
> > > >>
> > > >> This series also takes care of the backward compatibility by
> > > >> providing hardcoded values in case the new clocks are missing.
> > > >> So, existing DT should work.
> > > >>
> > > >>
> > > >> Changes in v2:
> > > >>   - Split patches into sensible chunks
> > > >>
> > > >> Masahiro Yamada (3):
> > > >>   mtd: rawnand: denali_dt: use dev as a shorthand of &pdev->dev
> > > >>   mtd: rawnand: denali_dt: add more clocks based on IP datasheet
> > > >>   mtd: rawnand: denali: optimize timing parameters for data interface    
> > > >
> > > > This still means that we need to feed at least 2/3 and 3/3 into -stable to
> > > > unbreak the driver.    
> > > 
> > > 
> > > 3/3 is not mandatory.
> > > 
> > > You can only back-port 1/3 and 2/3.  
> > 
> > Well, patch 1 is not a fix, can we move it after patch 2 so that only
> > patch 2 is flagged with the Fixes+Cc-stable tags?  
> 
> Another question, shall we fix arch/arm/boot/dts/socfpga.dtsi too?

Yep.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Masahiro Yamada June 14, 2018, 11:31 a.m. UTC | #6
Hi Boris,

2018-06-14 16:38 GMT+09:00 Boris Brezillon <boris.brezillon@bootlin.com>:
> On Thu, 14 Jun 2018 16:29:59 +0900
> Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
>
>> Hi Richard,
>>
>> 2018-06-14 16:25 GMT+09:00 Richard Weinberger <richard@sigma-star.at>:
>> > Masahiro,
>> >
>> > Am Donnerstag, 14. Juni 2018, 07:11:04 CEST schrieb Masahiro Yamada:
>> >>
>> >> The ->setup_data_interface() hook needs to know the clock frequency.
>> >> In fact, this IP needs three clocks, bus "which clock?" is really
>> >> confusing.  (It is not described in the DT-binding at all.)
>> >>
>> >> This series adds more clocks.  In the new binding, three clocks
>> >> are required: core clock, bus interface clock, ECC engine clock.
>> >>
>> >> This series also takes care of the backward compatibility by
>> >> providing hardcoded values in case the new clocks are missing.
>> >> So, existing DT should work.
>> >>
>> >>
>> >> Changes in v2:
>> >>   - Split patches into sensible chunks
>> >>
>> >> Masahiro Yamada (3):
>> >>   mtd: rawnand: denali_dt: use dev as a shorthand of &pdev->dev
>> >>   mtd: rawnand: denali_dt: add more clocks based on IP datasheet
>> >>   mtd: rawnand: denali: optimize timing parameters for data interface
>> >
>> > This still means that we need to feed at least 2/3 and 3/3 into -stable to
>> > unbreak the driver.
>>
>>
>> 3/3 is not mandatory.
>>
>> You can only back-port 1/3 and 2/3.
>
> Well, patch 1 is not a fix, can we move it after patch 2 so that only
> patch 2 is flagged with the Fixes+Cc-stable tags?


OK, will do that.

If you try to port this back to v4.14.*
you need to fix-up the file path
since the driver did not reside in the raw/ sub-directory at that time.


Except that, I do not see backport issue.




> Thanks,
>
> Boris
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
Boris Brezillon June 14, 2018, 11:38 a.m. UTC | #7
On Thu, 14 Jun 2018 20:31:59 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:

> Hi Boris,
> 
> 2018-06-14 16:38 GMT+09:00 Boris Brezillon <boris.brezillon@bootlin.com>:
> > On Thu, 14 Jun 2018 16:29:59 +0900
> > Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
> >  
> >> Hi Richard,
> >>
> >> 2018-06-14 16:25 GMT+09:00 Richard Weinberger <richard@sigma-star.at>:  
> >> > Masahiro,
> >> >
> >> > Am Donnerstag, 14. Juni 2018, 07:11:04 CEST schrieb Masahiro Yamada:  
> >> >>
> >> >> The ->setup_data_interface() hook needs to know the clock frequency.
> >> >> In fact, this IP needs three clocks, bus "which clock?" is really
> >> >> confusing.  (It is not described in the DT-binding at all.)
> >> >>
> >> >> This series adds more clocks.  In the new binding, three clocks
> >> >> are required: core clock, bus interface clock, ECC engine clock.
> >> >>
> >> >> This series also takes care of the backward compatibility by
> >> >> providing hardcoded values in case the new clocks are missing.
> >> >> So, existing DT should work.
> >> >>
> >> >>
> >> >> Changes in v2:
> >> >>   - Split patches into sensible chunks
> >> >>
> >> >> Masahiro Yamada (3):
> >> >>   mtd: rawnand: denali_dt: use dev as a shorthand of &pdev->dev
> >> >>   mtd: rawnand: denali_dt: add more clocks based on IP datasheet
> >> >>   mtd: rawnand: denali: optimize timing parameters for data interface  
> >> >
> >> > This still means that we need to feed at least 2/3 and 3/3 into -stable to
> >> > unbreak the driver.  
> >>
> >>
> >> 3/3 is not mandatory.
> >>
> >> You can only back-port 1/3 and 2/3.  
> >
> > Well, patch 1 is not a fix, can we move it after patch 2 so that only
> > patch 2 is flagged with the Fixes+Cc-stable tags?  
> 
> 
> OK, will do that.
> 
> If you try to port this back to v4.14.*
> you need to fix-up the file path
> since the driver did not reside in the raw/ sub-directory at that time.

Yes I know :-/. We'll do that when we receive GKH's notification
saying that the patch does not apply.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html