Hi,

This series adds a new SPI driver using the spi-mem interface for the
Aspeed static memory controllers of the AST2600, AST2500 and AST2400
SoCs.

 * AST2600 Firmware SPI Memory Controller (FMC)
 * AST2600 SPI Flash Controller (SPI1 and SPI2)
 * AST2500 Firmware SPI Memory Controller (FMC)
 * AST2500 SPI Flash Controller (SPI1 and SPI2)
 * AST2400 New Static Memory Controller (also referred as FMC)
 * AST2400 SPI Flash Controller (SPI)

It is based on the current OpenBMC kernel driver [1], using directly
the MTD SPI-NOR interface and on a patchset [2] previously proposed
adding support for the AST2600 only. This driver takes a slightly
different approach to cover all 6 controllers.

It does not make use of the controller register disabling Address and
Data byte lanes because is not available on the AST2400 SoC. We could
introduce a specific handler for new features available on recent SoCs
if needed. As there is not much difference on performance, the driver
chooses the common denominator: "User mode" which has been heavily
tested in [1]. "User mode" is also used as a fall back method when
flash device mapping window is too small.

Problems to address with spi-mem were the configuration of the mapping
windows and the calibration of the read timings. The driver handles
them in the direct mapping handler when some knowledge on the size of
the flash device is know. It is not perfect but not incorrect either.
The algorithm is one from [1] because it doesn't require the DMA
registers which are not available on all controllers.

Direct mapping for writes is not supported (yet). I have seen some
corruption with writes and I preferred to use the safer and proven
method of the initial driver [1]. We can improve that later.

The driver supports Quad SPI RX transfers on the AST2600 SoC but it
didn't have the expected results. Therefore it is not activated yet.
There are some issues on the pinctrl to investigate first. 

Tested on:
 
 * OpenPOWER Palmetto (AST2400)
 * Facebook Wedge 100 BMC (AST2400) by Tao Ren <rentao.bupt@gmail.com>
 * Evaluation board (AST2500) 
 * Inspur FP5280G2 BMC (AST2500) by John Wang <wangzq.jn@gmail.com>
 * Facebook Backpack CMM BMC (AST2500) by Tao Ren <rentao.bupt@gmail.com>
 * OpenPOWER Witherspoon (AST2500)
 * Evaluation board (AST2600 A0 and A3)
 * Rainier board (AST2600)
 
[1] https://github.com/openbmc/linux/blob/dev-5.15/drivers/mtd/spi-nor/controllers/aspeed-smc.c
[2] https://patchwork.ozlabs.org/project/linux-aspeed/list/?series=212394

Thanks,

C. 

Changes in v7:

  - Moved removal of the old binding desccription in the binding patch 

Changes in v6:

  - Removed tabs for aspeed,ast2600-fmc.yaml

Changes in v5:

  - Rebased on 5.18-rc5
  - More AST2600 tests from Jae Hyun Yoo
  - Cleanups of aspeed,ast2600-fmc.yaml
  - Modified aspeed_spi_send_cmd_addr() routine to return an error  
  - Simplified conditions in exec_op() handler when computing
    the controller setting
  - Dropped the use of memcpy_fromio for the SFDP address space

Changes in v4:

  - Rebased on 5.18 
  - Removal of the SPI-NOR base driver (we had enough tests)
  - Fix for small size flash devices on AST2600 (Potin)

Changes in v3:

 - Fixed compile warning on aspeed_spi_dirmap_read() prototype reported
   by kernel test robot 
 - Removed unnecessary entry in ast2600-fmc.yaml
 - New patch from Tao to set spi-max-frequency on all FMC devices

Changes in v2:

 - Fixed dt_binding_check warnings (Rob)
 - New entry in MAINTAINERS 
 - Addressed Lukas comments regarding the SPI controller registration
   and device removal. Checked with driver bind/unbind   
 - Introduced setup and cleanup handlers and removed routine looping
   on the DT children properties (Pratyush)
 - Clarified in commit log requirements for training.
 - Removed defconfig changes of patch 1 since they were reverted in
   the last patch (Joel)

Cédric Le Goater (9):
  ARM: dts: aspeed: Adjust "reg" property of FMC/SPI controllers
  dt-bindings: spi: Convert the Aspeed SMC controllers device tree
    binding
  spi: spi-mem: Convert Aspeed SMC driver to spi-mem
  spi: aspeed: Add support for direct mapping
  spi: aspeed: Adjust direct mapping to device size
  spi: aspeed: Workaround AST2500 limitations
  spi: aspeed: Add support for the AST2400 SPI controller
  spi: aspeed: Calibrate read timings
  ARM: dts: aspeed: Enable Dual SPI RX transfers

Potin Lai (1):
  mtd: spi-nor: aspeed: set the decoding size to at least 2MB for
    AST2600

Tao Ren (1):
  ARM: dts: aspeed-g4: Set spi-max-frequency for all flashes

 drivers/mtd/spi-nor/controllers/aspeed-smc.c  |  921 -------------
 drivers/spi/spi-aspeed-smc.c                  | 1210 +++++++++++++++++
 .../devicetree/bindings/mtd/aspeed-smc.txt    |   51 -
 .../bindings/spi/aspeed,ast2600-fmc.yaml      |   82 ++
 MAINTAINERS                                   |   10 +
 arch/arm/boot/dts/aspeed-g4.dtsi              |   16 +-
 arch/arm/boot/dts/aspeed-g5.dtsi              |   16 +-
 arch/arm/boot/dts/aspeed-g6.dtsi              |   17 +-
 drivers/mtd/spi-nor/controllers/Kconfig       |   10 -
 drivers/mtd/spi-nor/controllers/Makefile      |    1 -
 drivers/spi/Kconfig                           |   11 +
 drivers/spi/Makefile                          |    1 +
 12 files changed, 1347 insertions(+), 999 deletions(-)
 delete mode 100644 drivers/mtd/spi-nor/controllers/aspeed-smc.c
 create mode 100644 drivers/spi/spi-aspeed-smc.c
 delete mode 100644 Documentation/devicetree/bindings/mtd/aspeed-smc.txt
 create mode 100644 Documentation/devicetree/bindings/spi/aspeed,ast2600-fmc.yaml

-- 
2.35.1

On Mon, 9 May 2022 19:56:05 +0200, Cédric Le Goater wrote:
> This series adds a new SPI driver using the spi-mem interface for the
> Aspeed static memory controllers of the AST2600, AST2500 and AST2400
> SoCs.
> 
>  * AST2600 Firmware SPI Memory Controller (FMC)
>  * AST2600 SPI Flash Controller (SPI1 and SPI2)
>  * AST2500 Firmware SPI Memory Controller (FMC)
>  * AST2500 SPI Flash Controller (SPI1 and SPI2)
>  * AST2400 New Static Memory Controller (also referred as FMC)
>  * AST2400 SPI Flash Controller (SPI)
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[02/11] dt-bindings: spi: Convert the Aspeed SMC controllers device tree binding
        commit: ce9858ea499da025684a7a5f19823c2c3f14bdce
[03/11] spi: spi-mem: Convert Aspeed SMC driver to spi-mem
        commit: 9c63b846e6df43e5b3d31263f7db545f32deeda3
[04/11] spi: aspeed: Add support for direct mapping
        commit: 9da06d7bdec7dad8018c23b180e410ef2e7a4367
[05/11] spi: aspeed: Adjust direct mapping to device size
        commit: bb084f94e1bca4a5c4f689d7aa9b410220c1ed71
[06/11] spi: aspeed: Workaround AST2500 limitations
        commit: 5785eedee42c34cfec496199a80fa8ec9ddcf7fe
[07/11] spi: aspeed: Add support for the AST2400 SPI controller
        commit: 53526ab27d9c256504f267713aea60db7af18fb0
[08/11] spi: aspeed: Calibrate read timings
        commit: eeaec1ea05c0e0f08e04c6844f20cc24a2fcc0f4
[11/11] mtd: spi-nor: aspeed: set the decoding size to at least 2MB for AST2600
        commit: 73ae97e3cabb580639f02f12a192324a53c4bebb

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Hi Cedric,

On 16/05/22 07:39PM, Mark Brown wrote:
> On Mon, 9 May 2022 19:56:05 +0200, Cédric Le Goater wrote:
> > This series adds a new SPI driver using the spi-mem interface for the
> > Aspeed static memory controllers of the AST2600, AST2500 and AST2400
> > SoCs.
> > 
> >  * AST2600 Firmware SPI Memory Controller (FMC)
> >  * AST2600 SPI Flash Controller (SPI1 and SPI2)
> >  * AST2500 Firmware SPI Memory Controller (FMC)
> >  * AST2500 SPI Flash Controller (SPI1 and SPI2)
> >  * AST2400 New Static Memory Controller (also referred as FMC)
> >  * AST2400 SPI Flash Controller (SPI)
> > 
> > [...]
> 
> Applied to
> 
>    https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
> 
> Thanks!
> 
> [02/11] dt-bindings: spi: Convert the Aspeed SMC controllers device tree binding
>         commit: ce9858ea499da025684a7a5f19823c2c3f14bdce
> [03/11] spi: spi-mem: Convert Aspeed SMC driver to spi-mem
>         commit: 9c63b846e6df43e5b3d31263f7db545f32deeda3
> [04/11] spi: aspeed: Add support for direct mapping
>         commit: 9da06d7bdec7dad8018c23b180e410ef2e7a4367
> [05/11] spi: aspeed: Adjust direct mapping to device size
>         commit: bb084f94e1bca4a5c4f689d7aa9b410220c1ed71
> [06/11] spi: aspeed: Workaround AST2500 limitations
>         commit: 5785eedee42c34cfec496199a80fa8ec9ddcf7fe
> [07/11] spi: aspeed: Add support for the AST2400 SPI controller
>         commit: 53526ab27d9c256504f267713aea60db7af18fb0
> [08/11] spi: aspeed: Calibrate read timings
>         commit: eeaec1ea05c0e0f08e04c6844f20cc24a2fcc0f4

I have repeatedly objected to this patch [0][1][2] and you have 
repeatedly decided to not address my objections. I won't spend any more 
time fighting it. But I will say that you should not expect any 
guarantees that SPI NOR or SPI NAND will not break your calibration in 
the future if they decide to move the dirmap_create() call around.

> [11/11] mtd: spi-nor: aspeed: set the decoding size to at least 2MB for AST2600
>         commit: 73ae97e3cabb580639f02f12a192324a53c4bebb
> 

[0] https://patchwork.kernel.org/project/spi-devel-general/patch/20220325100849.2019209-9-clg@kaod.org/
[1] https://patchwork.kernel.org/project/spi-devel-general/patch/20220214094231.3753686-9-clg@kaod.org/
[2] https://lore.kernel.org/all/20220208190636.h6dubktkmuosvdxo@ti.com/

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

Pratyush,

On 5/17/22 13:05, Pratyush Yadav wrote:
> Hi Cedric,
> 
> On 16/05/22 07:39PM, Mark Brown wrote:
>> On Mon, 9 May 2022 19:56:05 +0200, Cédric Le Goater wrote:
>>> This series adds a new SPI driver using the spi-mem interface for the
>>> Aspeed static memory controllers of the AST2600, AST2500 and AST2400
>>> SoCs.
>>>
>>>   * AST2600 Firmware SPI Memory Controller (FMC)
>>>   * AST2600 SPI Flash Controller (SPI1 and SPI2)
>>>   * AST2500 Firmware SPI Memory Controller (FMC)
>>>   * AST2500 SPI Flash Controller (SPI1 and SPI2)
>>>   * AST2400 New Static Memory Controller (also referred as FMC)
>>>   * AST2400 SPI Flash Controller (SPI)
>>>
>>> [...]
>>
>> Applied to
>>
>>     https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
>>
>> Thanks!
>>
>> [02/11] dt-bindings: spi: Convert the Aspeed SMC controllers device tree binding
>>          commit: ce9858ea499da025684a7a5f19823c2c3f14bdce
>> [03/11] spi: spi-mem: Convert Aspeed SMC driver to spi-mem
>>          commit: 9c63b846e6df43e5b3d31263f7db545f32deeda3
>> [04/11] spi: aspeed: Add support for direct mapping
>>          commit: 9da06d7bdec7dad8018c23b180e410ef2e7a4367
>> [05/11] spi: aspeed: Adjust direct mapping to device size
>>          commit: bb084f94e1bca4a5c4f689d7aa9b410220c1ed71
>> [06/11] spi: aspeed: Workaround AST2500 limitations
>>          commit: 5785eedee42c34cfec496199a80fa8ec9ddcf7fe
>> [07/11] spi: aspeed: Add support for the AST2400 SPI controller
>>          commit: 53526ab27d9c256504f267713aea60db7af18fb0
>> [08/11] spi: aspeed: Calibrate read timings
>>          commit: eeaec1ea05c0e0f08e04c6844f20cc24a2fcc0f4
> 
> I have repeatedly objected to this patch [0][1][2] and you have
> repeatedly decided to not address my objections. 

That's a very harsh way of saying things. I did not decide anything
or ignore your comments. I answered your questions and acknowledged
that indeed the read training was done under the dirmap handler but
this was not a concern today since we had all the required information
from spimem.

We waited _together_, 5 or 6 weeks, for more inputs on how to address
the concerns you raised regarding the sustainability of this method.

> I won't spend any more time fighting it. 

This is not a fight. I don't know why you interpret it that way.

Now, since you object so explicitly, and since this patchset has
not reached the Linux kernel yet, we should consider dropping it.
I rather do that than push crap in mainline. But then, please,
provide solutions and not only objections !

> But I will say that you should not expect any
> guarantees that SPI NOR or SPI NAND will not break your calibration in
> the future if they decide to move the dirmap_create() call around.

If that's the case one day, we have multiple solutions :

   - stop doing the training
   - move the training to the appropriate handler if it exists
   - use a default value
  
>> [11/11] mtd: spi-nor: aspeed: set the decoding size to at least 2MB for AST2600
>>          commit: 73ae97e3cabb580639f02f12a192324a53c4bebb
>>
> 
> [0] https://patchwork.kernel.org/project/spi-devel-general/patch/20220325100849.2019209-9-clg@kaod.org/
> [1] https://patchwork.kernel.org/project/spi-devel-general/patch/20220214094231.3753686-9-clg@kaod.org/
> [2] https://lore.kernel.org/all/20220208190636.h6dubktkmuosvdxo@ti.com/

Regards,

Cédric.

Hi Cédric,

clg@kaod.org wrote on Tue, 17 May 2022 14:03:02 +0200:

> Pratyush,
> 
> On 5/17/22 13:05, Pratyush Yadav wrote:
> > Hi Cedric,
> > 
> > On 16/05/22 07:39PM, Mark Brown wrote:  
> >> On Mon, 9 May 2022 19:56:05 +0200, Cédric Le Goater wrote:  
> >>> This series adds a new SPI driver using the spi-mem interface for the
> >>> Aspeed static memory controllers of the AST2600, AST2500 and AST2400
> >>> SoCs.
> >>>
> >>>   * AST2600 Firmware SPI Memory Controller (FMC)
> >>>   * AST2600 SPI Flash Controller (SPI1 and SPI2)
> >>>   * AST2500 Firmware SPI Memory Controller (FMC)
> >>>   * AST2500 SPI Flash Controller (SPI1 and SPI2)
> >>>   * AST2400 New Static Memory Controller (also referred as FMC)
> >>>   * AST2400 SPI Flash Controller (SPI)
> >>>
> >>> [...]  
> >>
> >> Applied to
> >>
> >>     https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
> >>
> >> Thanks!
> >>
> >> [02/11] dt-bindings: spi: Convert the Aspeed SMC controllers device tree binding
> >>          commit: ce9858ea499da025684a7a5f19823c2c3f14bdce
> >> [03/11] spi: spi-mem: Convert Aspeed SMC driver to spi-mem
> >>          commit: 9c63b846e6df43e5b3d31263f7db545f32deeda3
> >> [04/11] spi: aspeed: Add support for direct mapping
> >>          commit: 9da06d7bdec7dad8018c23b180e410ef2e7a4367
> >> [05/11] spi: aspeed: Adjust direct mapping to device size
> >>          commit: bb084f94e1bca4a5c4f689d7aa9b410220c1ed71
> >> [06/11] spi: aspeed: Workaround AST2500 limitations
> >>          commit: 5785eedee42c34cfec496199a80fa8ec9ddcf7fe
> >> [07/11] spi: aspeed: Add support for the AST2400 SPI controller
> >>          commit: 53526ab27d9c256504f267713aea60db7af18fb0
> >> [08/11] spi: aspeed: Calibrate read timings
> >>          commit: eeaec1ea05c0e0f08e04c6844f20cc24a2fcc0f4  
> > 
> > I have repeatedly objected to this patch [0][1][2] and you have
> > repeatedly decided to not address my objections.   
> That's a very harsh way of saying things. I did not decide anything
> or ignore your comments. I answered your questions and acknowledged
> that indeed the read training was done under the dirmap handler but
> this was not a concern today since we had all the required information
> from spimem.
> 
> We waited _together_, 5 or 6 weeks, for more inputs on how to address
> the concerns you raised regarding the sustainability of this method.
> 
> > I won't spend any more time fighting it.   
> This is not a fight. I don't know why you interpret it that way.
> 
> Now, since you object so explicitly, and since this patchset has
> not reached the Linux kernel yet, we should consider dropping it.
> I rather do that than push crap in mainline. But then, please,
> provide solutions and not only objections !
> 
> > But I will say that you should not expect any
> > guarantees that SPI NOR or SPI NAND will not break your calibration in
> > the future if they decide to move the dirmap_create() call around.  
> 
> If that's the case one day, we have multiple solutions :
> 
>    - stop doing the training

Not really an option, isn't it?

>    - move the training to the appropriate handler if it exists

Sorry for jumping in so late, but why not using this solution already
if there are concerns about the current location? It does not look like
a complex solution to implement, but I might be overlooking something.

This can also be done for the next kernel release.

>    - use a default value
>   >> [11/11] mtd: spi-nor: aspeed: set the decoding size to at least 2MB for AST2600
> >>          commit: 73ae97e3cabb580639f02f12a192324a53c4bebb
> >>  
> > 
> > [0] https://patchwork.kernel.org/project/spi-devel-general/patch/20220325100849.2019209-9-clg@kaod.org/
> > [1] https://patchwork.kernel.org/project/spi-devel-general/patch/20220214094231.3753686-9-clg@kaod.org/
> > [2] https://lore.kernel.org/all/20220208190636.h6dubktkmuosvdxo@ti.com/  
> 
> Regards,
> 
> Cédric.


Thanks,
Miquèl

Hello Miquel,

On 5/17/22 14:37, Miquel Raynal wrote:
> Hi Cédric,
> 
> clg@kaod.org wrote on Tue, 17 May 2022 14:03:02 +0200:
> 
>> Pratyush,
>>
>> On 5/17/22 13:05, Pratyush Yadav wrote:
>>> Hi Cedric,
>>>
>>> On 16/05/22 07:39PM, Mark Brown wrote:
>>>> On Mon, 9 May 2022 19:56:05 +0200, Cédric Le Goater wrote:
>>>>> This series adds a new SPI driver using the spi-mem interface for the
>>>>> Aspeed static memory controllers of the AST2600, AST2500 and AST2400
>>>>> SoCs.
>>>>>
>>>>>    * AST2600 Firmware SPI Memory Controller (FMC)
>>>>>    * AST2600 SPI Flash Controller (SPI1 and SPI2)
>>>>>    * AST2500 Firmware SPI Memory Controller (FMC)
>>>>>    * AST2500 SPI Flash Controller (SPI1 and SPI2)
>>>>>    * AST2400 New Static Memory Controller (also referred as FMC)
>>>>>    * AST2400 SPI Flash Controller (SPI)
>>>>>
>>>>> [...]
>>>>
>>>> Applied to
>>>>
>>>>      https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
>>>>
>>>> Thanks!
>>>>
>>>> [02/11] dt-bindings: spi: Convert the Aspeed SMC controllers device tree binding
>>>>           commit: ce9858ea499da025684a7a5f19823c2c3f14bdce
>>>> [03/11] spi: spi-mem: Convert Aspeed SMC driver to spi-mem
>>>>           commit: 9c63b846e6df43e5b3d31263f7db545f32deeda3
>>>> [04/11] spi: aspeed: Add support for direct mapping
>>>>           commit: 9da06d7bdec7dad8018c23b180e410ef2e7a4367
>>>> [05/11] spi: aspeed: Adjust direct mapping to device size
>>>>           commit: bb084f94e1bca4a5c4f689d7aa9b410220c1ed71
>>>> [06/11] spi: aspeed: Workaround AST2500 limitations
>>>>           commit: 5785eedee42c34cfec496199a80fa8ec9ddcf7fe
>>>> [07/11] spi: aspeed: Add support for the AST2400 SPI controller
>>>>           commit: 53526ab27d9c256504f267713aea60db7af18fb0
>>>> [08/11] spi: aspeed: Calibrate read timings
>>>>           commit: eeaec1ea05c0e0f08e04c6844f20cc24a2fcc0f4
>>>
>>> I have repeatedly objected to this patch [0][1][2] and you have
>>> repeatedly decided to not address my objections.
>> That's a very harsh way of saying things. I did not decide anything
>> or ignore your comments. I answered your questions and acknowledged
>> that indeed the read training was done under the dirmap handler but
>> this was not a concern today since we had all the required information
>> from spimem.
>>
>> We waited _together_, 5 or 6 weeks, for more inputs on how to address
>> the concerns you raised regarding the sustainability of this method.
>>
>>> I won't spend any more time fighting it.
>> This is not a fight. I don't know why you interpret it that way.
>>
>> Now, since you object so explicitly, and since this patchset has
>> not reached the Linux kernel yet, we should consider dropping it.
>> I rather do that than push crap in mainline. But then, please,
>> provide solutions and not only objections !
>>
>>> But I will say that you should not expect any
>>> guarantees that SPI NOR or SPI NAND will not break your calibration in
>>> the future if they decide to move the dirmap_create() call around.
>>
>> If that's the case one day, we have multiple solutions :
>>
>>     - stop doing the training
> 
> Not really an option, isn't it?

well, I would really rather keep the training. Else the device would be
crippled and run at low speed. 1/16 HCLK is the default :/

> 
>>     - move the training to the appropriate handler if it exists
> 
> Sorry for jumping in so late, but why not using this solution already
> if there are concerns about the current location? It does not look like
> a complex solution to implement, but I might be overlooking something.

I think we need some more feedback on this proposal [0].

If that's not acceptable, then let's discuss on dirmap_create() and
see if it would be acceptable to do calibration under this hook.
Which is what this patchset is proposing since the spimem interface
offers all the required information to do so.

However, Pratyush would prefer a stable interface, hence [0].

That said, if one day we merge a do_calibration() hook like proposed
in [0], we can change the current Aspeed patchset to use it. That's
why I didn't see the lack of specific calibration hook as a blocker.

> This can also be done for the next kernel release.

I was hoping this one. We would need a comment on the use of the
dirmap_create() hook to clarify.

Thanks !

Cédric

[0] https://patchwork.ozlabs.org/project/linux-mtd/list/?series=233504&state=%2A&archive=both

Hi Cedric,

On 17/05/22 02:03PM, Cédric Le Goater wrote:
> Pratyush,
> 
> On 5/17/22 13:05, Pratyush Yadav wrote:
> > Hi Cedric,
> > 
> > On 16/05/22 07:39PM, Mark Brown wrote:
> > > On Mon, 9 May 2022 19:56:05 +0200, Cédric Le Goater wrote:
> > > > This series adds a new SPI driver using the spi-mem interface for the
> > > > Aspeed static memory controllers of the AST2600, AST2500 and AST2400
> > > > SoCs.
> > > > 
> > > >   * AST2600 Firmware SPI Memory Controller (FMC)
> > > >   * AST2600 SPI Flash Controller (SPI1 and SPI2)
> > > >   * AST2500 Firmware SPI Memory Controller (FMC)
> > > >   * AST2500 SPI Flash Controller (SPI1 and SPI2)
> > > >   * AST2400 New Static Memory Controller (also referred as FMC)
> > > >   * AST2400 SPI Flash Controller (SPI)
> > > > 
> > > > [...]
> > > 
> > > Applied to
> > > 
> > >     https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
> > > 
> > > Thanks!
> > > 
> > > [02/11] dt-bindings: spi: Convert the Aspeed SMC controllers device tree binding
> > >          commit: ce9858ea499da025684a7a5f19823c2c3f14bdce
> > > [03/11] spi: spi-mem: Convert Aspeed SMC driver to spi-mem
> > >          commit: 9c63b846e6df43e5b3d31263f7db545f32deeda3
> > > [04/11] spi: aspeed: Add support for direct mapping
> > >          commit: 9da06d7bdec7dad8018c23b180e410ef2e7a4367
> > > [05/11] spi: aspeed: Adjust direct mapping to device size
> > >          commit: bb084f94e1bca4a5c4f689d7aa9b410220c1ed71
> > > [06/11] spi: aspeed: Workaround AST2500 limitations
> > >          commit: 5785eedee42c34cfec496199a80fa8ec9ddcf7fe
> > > [07/11] spi: aspeed: Add support for the AST2400 SPI controller
> > >          commit: 53526ab27d9c256504f267713aea60db7af18fb0
> > > [08/11] spi: aspeed: Calibrate read timings
> > >          commit: eeaec1ea05c0e0f08e04c6844f20cc24a2fcc0f4
> > 
> > I have repeatedly objected to this patch [0][1][2] and you have
> > repeatedly decided to not address my objections.
> 
> That's a very harsh way of saying things. I did not decide anything
> or ignore your comments. I answered your questions and acknowledged
> that indeed the read training was done under the dirmap handler but
> this was not a concern today since we had all the required information
> from spimem.

I think the correct thing to do here would be to move this patch out of 
your main series. Since the old driver did not have this feature anyway, 
it is not necessary for the conversion to achieve feature parity. Then 
have a separate series to drive the conversation on how to best 
implement this contentious feature.

> 
> We waited _together_, 5 or 6 weeks, for more inputs on how to address
> the concerns you raised regarding the sustainability of this method.
> 
> > I won't spend any more time fighting it.
> 
> This is not a fight. I don't know why you interpret it that way.

I interpret it this way because I feel you have ignored my comments and 
have kept this patch in regardless. But maybe I _am_ being too harsh. So 
apologies if I have interpreted things wrong.

> 
> Now, since you object so explicitly, and since this patchset has
> not reached the Linux kernel yet, we should consider dropping it.
> I rather do that than push crap in mainline. But then, please,
> provide solutions and not only objections !

Yes please. I would like to have this patch dropped, and then reopen 
this discussion in a separate thread.

I have provided solutions before but maybe you missed them. Please see 
[3]. Specifically patches 1 and 3. These are still not very refined and 
might not be accepted in their current state (especially patch 3 since 
it does the calibration after registering the device with MTD), but they 
should be a good starting point for discussion.

And we might end up deciding to just use the dirmap_create() hook to do 
the calibration, but it is important to have this conversation 
separately and clearly instead of it being missed as part of a large 
patch series.

> 
> > But I will say that you should not expect any
> > guarantees that SPI NOR or SPI NAND will not break your calibration in
> > the future if they decide to move the dirmap_create() call around.
> 
> If that's the case one day, we have multiple solutions :
> 
>   - stop doing the training
>   - move the training to the appropriate handler if it exists

This is my preferred option, [3] is a good starting point for proposing 
that handler.

>   - use a default value
> > > [11/11] mtd: spi-nor: aspeed: set the decoding size to at least 2MB for AST2600
> > >          commit: 73ae97e3cabb580639f02f12a192324a53c4bebb
> > > 
> > 
> > [0] https://patchwork.kernel.org/project/spi-devel-general/patch/20220325100849.2019209-9-clg@kaod.org/
> > [1] https://patchwork.kernel.org/project/spi-devel-general/patch/20220214094231.3753686-9-clg@kaod.org/
> > [2] https://lore.kernel.org/all/20220208190636.h6dubktkmuosvdxo@ti.com/
[3] https://patchwork.ozlabs.org/project/linux-mtd/list/?series=233504&state=%2A&archive=both

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

On Tue, May 17, 2022 at 04:35:09PM +0530, Pratyush Yadav wrote:
> On 16/05/22 07:39PM, Mark Brown wrote:
> > On Mon, 9 May 2022 19:56:05 +0200, Cédric Le Goater wrote:

> > [08/11] spi: aspeed: Calibrate read timings
> >         commit: eeaec1ea05c0e0f08e04c6844f20cc24a2fcc0f4

> I have repeatedly objected to this patch [0][1][2] and you have 
> repeatedly decided to not address my objections. I won't spend any more 
> time fighting it. But I will say that you should not expect any 
> guarantees that SPI NOR or SPI NAND will not break your calibration in 
> the future if they decide to move the dirmap_create() call around.

You == Cedric here.  Sorry, I thought that had been addressed as there
hadn't been anything I'd noticed on the thread for a few versions.  It
does look like there was agreement that there was a problem too :/