Re: [PATCH v10 12/27] mtd: spi-nor-core: Rework hwcaps selection

2021-07-28 Thread Bin Meng
Hi Pratyush,

On Wed, Jul 28, 2021 at 6:44 PM Pratyush Yadav  wrote:
>
> On 28/07/21 06:13PM, Bin Meng wrote:
> > Hi Pratyush,
> >
> > On Sat, Jun 26, 2021 at 3:20 AM Pratyush Yadav  wrote:
> > >
> > > The spi-mem layer provides a spi_mem_supports_op() function to check
> > > whether a specific operation is supported by the controller or not.
> > > This is much more accurate than the hwcaps selection logic based on
> > > SPI_{RX,TX}_ flags.
> > >
> > > Rework the hwcaps selection logic to use spi_mem_supports_op().
> > >
> > > To make sure the build doesn't break for boards not using CONFIG_DM_SPI,
> > > add a simple SPI_{RX,TX}_ based hwcaps selection logic in spi-mem-nodm
> > > similar to spi_mem_default_supports_op(). This change is only
> > > compile-tested.
> > >
> > > To avoid SPL size problems on the x530 board, the old hwcaps selection
> > > is still kept around. Leaving the code in-place was getting difficult to
> > > read and understand, so the code is restructured to have it all in one
> > > isolated function. As a result of this, the parameter hwcaps to
> > > spi_nor_setup() is no longer needed. Remove it.
> > >
> > > Based on the Linux commit c76f5089796a (mtd: spi-nor: Rework hwcaps
> > > selection for the spi-mem case, 2019-08-06)
> > >
> > > Signed-off-by: Pratyush Yadav 
> > > ---
> > >  drivers/mtd/spi/Kconfig|   9 ++
> > >  drivers/mtd/spi/spi-nor-core.c | 244 ++---
> > >  drivers/spi/spi-mem-nodm.c |  62 +
> > >  include/linux/mtd/spi-nor.h|  17 ++-
> > >  4 files changed, 280 insertions(+), 52 deletions(-)
> > >
> > > diff --git a/drivers/mtd/spi/Kconfig b/drivers/mtd/spi/Kconfig
> > > index f8db8e5213..a701167dcc 100644
> > > --- a/drivers/mtd/spi/Kconfig
> > > +++ b/drivers/mtd/spi/Kconfig
> > > @@ -88,6 +88,15 @@ config SPI_FLASH_SFDP_SUPPORT
> > >  SPI NOR flashes using Serial Flash Discoverable Parameters (SFDP)
> > >  tables as per JESD216 standard.
> > >
> > > +config SPI_FLASH_SMART_HWCAPS
> > > +   bool "Smart hardware capability detection based on SPI MEM 
> > > supports_op() hook"
> > > +   default y
> >
> > This commit unfortunately breaks ICH SPI driver working with SPI
> > flashes on Intel boards.
> >
> > Switching off this option makes it work again.
> >
> > I think this is due to that ICH SPI driver has set the flags
> > SPI_RX_SLOW | SPI_TX_BYTE but these 2 flags are not handled in the new
> > codes.
> > Any quick ideas on how to fix this?
>
> So can the controller can only support 1S-1S-1S mode? Or can the
> controller support more modes but the board does not have the other
> lines connected?
>
> For the former case, you should populate the supports_op() hook and
> reject ops that are not 1S-1S-1S. For the latter case, you should set
> spi-{rx,tx}-bus-width = <1> in device tree.

Thank you. I just sent a RFC patch that fixed this issue.

>
> Make sure you call spi_mem_default_supports_op() from your supports_op()
> hook otherwise the slave->mode bits are not taken into account.
>
> >
> > > +   help
> > > +Enable support for smart hardware capability detection based on 
> > > SPI
> > > +MEM supports_op() hook that lets controllers express whether they
> > > +can support a type of operation in a much more refined way 
> > > compared
> > > +to using flags like SPI_RX_DUAL, SPI_TX_QUAD, etc.
> > > +
> >
> > [snip]
> >

Regards,
Bin


Re: [PATCH v10 12/27] mtd: spi-nor-core: Rework hwcaps selection

2021-07-28 Thread Pratyush Yadav
On 28/07/21 06:13PM, Bin Meng wrote:
> Hi Pratyush,
> 
> On Sat, Jun 26, 2021 at 3:20 AM Pratyush Yadav  wrote:
> >
> > The spi-mem layer provides a spi_mem_supports_op() function to check
> > whether a specific operation is supported by the controller or not.
> > This is much more accurate than the hwcaps selection logic based on
> > SPI_{RX,TX}_ flags.
> >
> > Rework the hwcaps selection logic to use spi_mem_supports_op().
> >
> > To make sure the build doesn't break for boards not using CONFIG_DM_SPI,
> > add a simple SPI_{RX,TX}_ based hwcaps selection logic in spi-mem-nodm
> > similar to spi_mem_default_supports_op(). This change is only
> > compile-tested.
> >
> > To avoid SPL size problems on the x530 board, the old hwcaps selection
> > is still kept around. Leaving the code in-place was getting difficult to
> > read and understand, so the code is restructured to have it all in one
> > isolated function. As a result of this, the parameter hwcaps to
> > spi_nor_setup() is no longer needed. Remove it.
> >
> > Based on the Linux commit c76f5089796a (mtd: spi-nor: Rework hwcaps
> > selection for the spi-mem case, 2019-08-06)
> >
> > Signed-off-by: Pratyush Yadav 
> > ---
> >  drivers/mtd/spi/Kconfig|   9 ++
> >  drivers/mtd/spi/spi-nor-core.c | 244 ++---
> >  drivers/spi/spi-mem-nodm.c |  62 +
> >  include/linux/mtd/spi-nor.h|  17 ++-
> >  4 files changed, 280 insertions(+), 52 deletions(-)
> >
> > diff --git a/drivers/mtd/spi/Kconfig b/drivers/mtd/spi/Kconfig
> > index f8db8e5213..a701167dcc 100644
> > --- a/drivers/mtd/spi/Kconfig
> > +++ b/drivers/mtd/spi/Kconfig
> > @@ -88,6 +88,15 @@ config SPI_FLASH_SFDP_SUPPORT
> >  SPI NOR flashes using Serial Flash Discoverable Parameters (SFDP)
> >  tables as per JESD216 standard.
> >
> > +config SPI_FLASH_SMART_HWCAPS
> > +   bool "Smart hardware capability detection based on SPI MEM 
> > supports_op() hook"
> > +   default y
> 
> This commit unfortunately breaks ICH SPI driver working with SPI
> flashes on Intel boards.
> 
> Switching off this option makes it work again.
> 
> I think this is due to that ICH SPI driver has set the flags
> SPI_RX_SLOW | SPI_TX_BYTE but these 2 flags are not handled in the new
> codes.
> Any quick ideas on how to fix this?

So can the controller can only support 1S-1S-1S mode? Or can the 
controller support more modes but the board does not have the other 
lines connected?

For the former case, you should populate the supports_op() hook and 
reject ops that are not 1S-1S-1S. For the latter case, you should set 
spi-{rx,tx}-bus-width = <1> in device tree.

Make sure you call spi_mem_default_supports_op() from your supports_op() 
hook otherwise the slave->mode bits are not taken into account.

> 
> > +   help
> > +Enable support for smart hardware capability detection based on SPI
> > +MEM supports_op() hook that lets controllers express whether they
> > +can support a type of operation in a much more refined way compared
> > +to using flags like SPI_RX_DUAL, SPI_TX_QUAD, etc.
> > +
> 
> [snip]
> 
> Regards,
> Bin

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.


Re: [PATCH v10 12/27] mtd: spi-nor-core: Rework hwcaps selection

2021-07-28 Thread Bin Meng
Hi Pratyush,

On Sat, Jun 26, 2021 at 3:20 AM Pratyush Yadav  wrote:
>
> The spi-mem layer provides a spi_mem_supports_op() function to check
> whether a specific operation is supported by the controller or not.
> This is much more accurate than the hwcaps selection logic based on
> SPI_{RX,TX}_ flags.
>
> Rework the hwcaps selection logic to use spi_mem_supports_op().
>
> To make sure the build doesn't break for boards not using CONFIG_DM_SPI,
> add a simple SPI_{RX,TX}_ based hwcaps selection logic in spi-mem-nodm
> similar to spi_mem_default_supports_op(). This change is only
> compile-tested.
>
> To avoid SPL size problems on the x530 board, the old hwcaps selection
> is still kept around. Leaving the code in-place was getting difficult to
> read and understand, so the code is restructured to have it all in one
> isolated function. As a result of this, the parameter hwcaps to
> spi_nor_setup() is no longer needed. Remove it.
>
> Based on the Linux commit c76f5089796a (mtd: spi-nor: Rework hwcaps
> selection for the spi-mem case, 2019-08-06)
>
> Signed-off-by: Pratyush Yadav 
> ---
>  drivers/mtd/spi/Kconfig|   9 ++
>  drivers/mtd/spi/spi-nor-core.c | 244 ++---
>  drivers/spi/spi-mem-nodm.c |  62 +
>  include/linux/mtd/spi-nor.h|  17 ++-
>  4 files changed, 280 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/mtd/spi/Kconfig b/drivers/mtd/spi/Kconfig
> index f8db8e5213..a701167dcc 100644
> --- a/drivers/mtd/spi/Kconfig
> +++ b/drivers/mtd/spi/Kconfig
> @@ -88,6 +88,15 @@ config SPI_FLASH_SFDP_SUPPORT
>  SPI NOR flashes using Serial Flash Discoverable Parameters (SFDP)
>  tables as per JESD216 standard.
>
> +config SPI_FLASH_SMART_HWCAPS
> +   bool "Smart hardware capability detection based on SPI MEM 
> supports_op() hook"
> +   default y

This commit unfortunately breaks ICH SPI driver working with SPI
flashes on Intel boards.

Switching off this option makes it work again.

I think this is due to that ICH SPI driver has set the flags
SPI_RX_SLOW | SPI_TX_BYTE but these 2 flags are not handled in the new
codes.
Any quick ideas on how to fix this?

> +   help
> +Enable support for smart hardware capability detection based on SPI
> +MEM supports_op() hook that lets controllers express whether they
> +can support a type of operation in a much more refined way compared
> +to using flags like SPI_RX_DUAL, SPI_TX_QUAD, etc.
> +

[snip]

Regards,
Bin