[PATCH v2 0/2] spi: npcm-pspi: add Arbel NPCM8XX support

[PATCH v2 0/2] spi: npcm-pspi: add Arbel NPCM8XX support

[Tomer Maimon]

This patch set adds Arbel NPCM8XX Peripheral SPI (PSPI) support to PSPI NPCM
driver.

This patch set was separated from the full duplex patch.
https://lore.kernel.org/lkml/YtlES7MX6nJr8l+L@sirena.org.uk/

Addressed comments from:
 - Mark Brown : https://www.spinics.net/lists/kernel/msg4447178.html

Changes since version 1:
 - Adding comptible npcm845 in the driver. 

The NPCM PSPI driver tested on NPCM845 evaluation board.

Tomer Maimon (2):
  dt-binding: spi: npcm-pspi: Add npcm845 compatible
  spi: npcm-pspi: Add NPCM845 peripheral SPI support

 Documentation/devicetree/bindings/spi/nuvoton,npcm-pspi.txt | 3 ++-
 drivers/spi/spi-npcm-pspi.c                                 | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

-- 
2.33.0

[PATCH v2 1/2] dt-binding: spi: npcm-pspi: Add npcm845 compatible

[Tomer Maimon]

Add a compatible string for Nuvoton BMC NPCM845 PSPI.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 Documentation/devicetree/bindings/spi/nuvoton,npcm-pspi.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/spi/nuvoton,npcm-pspi.txt b/Documentation/devicetree/bindings/spi/nuvoton,npcm-pspi.txt
index b98203ca656d..a4e72e52af59 100644
--- a/Documentation/devicetree/bindings/spi/nuvoton,npcm-pspi.txt
+++ b/Documentation/devicetree/bindings/spi/nuvoton,npcm-pspi.txt
@@ -3,7 +3,8 @@ Nuvoton NPCM Peripheral Serial Peripheral Interface(PSPI) controller driver
 Nuvoton NPCM7xx SOC support two PSPI channels.
 
 Required properties:
- - compatible : "nuvoton,npcm750-pspi" for NPCM7XX BMC
+ - compatible : "nuvoton,npcm750-pspi" for Poleg NPCM7XX.
+				"nuvoton,npcm845-pspi" for Arbel NPCM8XX.
  - #address-cells : should be 1. see spi-bus.txt
  - #size-cells : should be 0. see spi-bus.txt
  - specifies physical base address and size of the register.
-- 
2.33.0

[PATCH v2 2/2] spi: npcm-pspi: Add NPCM845 peripheral SPI support

[Tomer Maimon]

Add Nuvoton BMC NPCM845 NPCM Peripheral SPI (PSPI) support.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 drivers/spi/spi-npcm-pspi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/spi/spi-npcm-pspi.c b/drivers/spi/spi-npcm-pspi.c
index 1668a347e003..7f2e4d1b0d43 100644
--- a/drivers/spi/spi-npcm-pspi.c
+++ b/drivers/spi/spi-npcm-pspi.c
@@ -443,6 +443,7 @@ static int npcm_pspi_remove(struct platform_device *pdev)
 
 static const struct of_device_id npcm_pspi_match[] = {
 	{ .compatible = "nuvoton,npcm750-pspi", .data = NULL },
+	{ .compatible = "nuvoton,npcm845-pspi", .data = NULL },
 	{}
 };
 MODULE_DEVICE_TABLE(of, npcm_pspi_match);
-- 
2.33.0

Re: [PATCH v2 2/2] spi: npcm-pspi: Add NPCM845 peripheral SPI support

[Krzysztof Kozlowski]

On 22/07/2022 13:41, Tomer Maimon wrote:
> Add Nuvoton BMC NPCM845 NPCM Peripheral SPI (PSPI) support.
> 
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> ---
>  drivers/spi/spi-npcm-pspi.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/spi/spi-npcm-pspi.c b/drivers/spi/spi-npcm-pspi.c
> index 1668a347e003..7f2e4d1b0d43 100644
> --- a/drivers/spi/spi-npcm-pspi.c
> +++ b/drivers/spi/spi-npcm-pspi.c
> @@ -443,6 +443,7 @@ static int npcm_pspi_remove(struct platform_device *pdev)
>  
>  static const struct of_device_id npcm_pspi_match[] = {
>  	{ .compatible = "nuvoton,npcm750-pspi", .data = NULL },
> +	{ .compatible = "nuvoton,npcm845-pspi", .data = NULL },

The devices look compatible, so why not reusing 750 compatible and
adding more specific upfront only in the bindings instead?

Best regards,
Krzysztof

Re: [PATCH v2 2/2] spi: npcm-pspi: Add NPCM845 peripheral SPI support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 688 bytes --]

On Fri, Jul 22, 2022 at 08:31:22PM +0200, Krzysztof Kozlowski wrote:
> On 22/07/2022 13:41, Tomer Maimon wrote:

> >  static const struct of_device_id npcm_pspi_match[] = {
> >  	{ .compatible = "nuvoton,npcm750-pspi", .data = NULL },
> > +	{ .compatible = "nuvoton,npcm845-pspi", .data = NULL },

> The devices look compatible, so why not reusing 750 compatible and
> adding more specific upfront only in the bindings instead?

...with a fallback list required by the bindings so the driver actually
binds.  Note that bindings are currently not in YAML format so there'd
be even less enforcement of that than normal, and as they're currently
written the bindings don't require fallback.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 2/2] spi: npcm-pspi: Add NPCM845 peripheral SPI support

[Krzysztof Kozlowski]

On 22/07/2022 20:43, Mark Brown wrote:
> On Fri, Jul 22, 2022 at 08:31:22PM +0200, Krzysztof Kozlowski wrote:
>> On 22/07/2022 13:41, Tomer Maimon wrote:
> 
>>>  static const struct of_device_id npcm_pspi_match[] = {
>>>  	{ .compatible = "nuvoton,npcm750-pspi", .data = NULL },
>>> +	{ .compatible = "nuvoton,npcm845-pspi", .data = NULL },
> 
>> The devices look compatible, so why not reusing 750 compatible and
>> adding more specific upfront only in the bindings instead?
> 
> ...with a fallback list required by the bindings so the driver actually
> binds.  Note that bindings are currently not in YAML format so there'd
> be even less enforcement of that than normal, and as they're currently
> written the bindings don't require fallback.

Yes, the bindings document should be rephrased but we were living like
that for few years. :)


Best regards,
Krzysztof

Re: [PATCH v2 2/2] spi: npcm-pspi: Add NPCM845 peripheral SPI support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 923 bytes --]

On Fri, Jul 22, 2022 at 08:47:24PM +0200, Krzysztof Kozlowski wrote:
> On 22/07/2022 20:43, Mark Brown wrote:

> > ...with a fallback list required by the bindings so the driver actually
> > binds.  Note that bindings are currently not in YAML format so there'd
> > be even less enforcement of that than normal, and as they're currently
> > written the bindings don't require fallback.

> Yes, the bindings document should be rephrased but we were living like
> that for few years. :)

The binding document as it stands only has one compatible, there's no
existing problem with it other than the YAML conversion.  If we're
adding something new that requires a fallback we should be explicit
about that rather than have something that's actively misleading where
previously things were clear.  I don't mind if we add the compatible to
the driver or document the requirement for the fallback but we should do
one of the two.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 2/2] spi: npcm-pspi: Add NPCM845 peripheral SPI support

[Tomer Maimon]

Hi Mark and Krzysztof,

Thanks for your reply,

On Fri, 22 Jul 2022 at 21:57, Mark Brown <broonie@kernel.org> wrote:
>
> On Fri, Jul 22, 2022 at 08:47:24PM +0200, Krzysztof Kozlowski wrote:
> > On 22/07/2022 20:43, Mark Brown wrote:
>
> > > ...with a fallback list required by the bindings so the driver actually
> > > binds.  Note that bindings are currently not in YAML format so there'd
> > > be even less enforcement of that than normal, and as they're currently
> > > written the bindings don't require fallback.
>
> > Yes, the bindings document should be rephrased but we were living like
> > that for few years. :)
>
> The binding document as it stands only has one compatible, there's no
> existing problem with it other than the YAML conversion.  If we're
> adding something new that requires a fallback we should be explicit
> about that rather than have something that's actively misleading where
> previously things were clear.  I don't mind if we add the compatible to
> the driver or document the requirement for the fallback but we should do
> one of the two.

is V2 good enough? adding the compatible to the driver and the document?
Or should we use fallback?
If fallback is choosen, can you explain how I should do it?

Best regards,

Tomer

Re: [PATCH v2 1/2] dt-binding: spi: npcm-pspi: Add npcm845 compatible

[Rob Herring]

On Fri, Jul 22, 2022 at 02:41:35PM +0300, Tomer Maimon wrote:
> Add a compatible string for Nuvoton BMC NPCM845 PSPI.
> 
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> ---
>  Documentation/devicetree/bindings/spi/nuvoton,npcm-pspi.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/nuvoton,npcm-pspi.txt b/Documentation/devicetree/bindings/spi/nuvoton,npcm-pspi.txt
> index b98203ca656d..a4e72e52af59 100644
> --- a/Documentation/devicetree/bindings/spi/nuvoton,npcm-pspi.txt
> +++ b/Documentation/devicetree/bindings/spi/nuvoton,npcm-pspi.txt
> @@ -3,7 +3,8 @@ Nuvoton NPCM Peripheral Serial Peripheral Interface(PSPI) controller driver
>  Nuvoton NPCM7xx SOC support two PSPI channels.
>  
>  Required properties:
> - - compatible : "nuvoton,npcm750-pspi" for NPCM7XX BMC
> + - compatible : "nuvoton,npcm750-pspi" for Poleg NPCM7XX.

Who/What is Polog? Board/BMC vendor? If so, that should not be in SoC 
bindings.

> +				"nuvoton,npcm845-pspi" for Arbel NPCM8XX.

Same for Arbel.

To answer your question on fallback, just put:

    "nuvoton,npcm845-pspi", "nuvoton,npcm750-pspi" for NPCM8XX BMC


>   - #address-cells : should be 1. see spi-bus.txt
>   - #size-cells : should be 0. see spi-bus.txt
>   - specifies physical base address and size of the register.
> -- 
> 2.33.0
> 
> 

Re: [PATCH v2 1/2] dt-binding: spi: npcm-pspi: Add npcm845 compatible

[Tomer Maimon]

Hi Rob,

Thanks for your comments.

On Tue, 26 Jul 2022 at 01:56, Rob Herring <robh@kernel.org> wrote:
>
> On Fri, Jul 22, 2022 at 02:41:35PM +0300, Tomer Maimon wrote:
> > Add a compatible string for Nuvoton BMC NPCM845 PSPI.
> >
> > Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/spi/nuvoton,npcm-pspi.txt | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/spi/nuvoton,npcm-pspi.txt b/Documentation/devicetree/bindings/spi/nuvoton,npcm-pspi.txt
> > index b98203ca656d..a4e72e52af59 100644
> > --- a/Documentation/devicetree/bindings/spi/nuvoton,npcm-pspi.txt
> > +++ b/Documentation/devicetree/bindings/spi/nuvoton,npcm-pspi.txt
> > @@ -3,7 +3,8 @@ Nuvoton NPCM Peripheral Serial Peripheral Interface(PSPI) controller driver
> >  Nuvoton NPCM7xx SOC support two PSPI channels.
> >
> >  Required properties:
> > - - compatible : "nuvoton,npcm750-pspi" for NPCM7XX BMC
> > + - compatible : "nuvoton,npcm750-pspi" for Poleg NPCM7XX.
>
> Who/What is Polog? Board/BMC vendor? If so, that should not be in SoC
> bindings.
>
> > +                             "nuvoton,npcm845-pspi" for Arbel NPCM8XX.
>
> Same for Arbel.
Poleg and Arbel are the name of the BMC SoC, will be removed next version.
>
> To answer your question on fallback, just put:
>
>     "nuvoton,npcm845-pspi", "nuvoton,npcm750-pspi" for NPCM8XX BMC
In the document?
because I don't remember doing so in other documents that I use fallback.
I need to do it in ymal files as well?
>
>
> >   - #address-cells : should be 1. see spi-bus.txt
> >   - #size-cells : should be 0. see spi-bus.txt
> >   - specifies physical base address and size of the register.
> > --
> > 2.33.0
> >
> >

Best regards,

Tomer

Re: [PATCH v2 2/2] spi: npcm-pspi: Add NPCM845 peripheral SPI support

[Krzysztof Kozlowski]

On 24/07/2022 10:44, Tomer Maimon wrote:
> Hi Mark and Krzysztof,
> 
> Thanks for your reply,
> 
> On Fri, 22 Jul 2022 at 21:57, Mark Brown <broonie@kernel.org> wrote:
>>
>> On Fri, Jul 22, 2022 at 08:47:24PM +0200, Krzysztof Kozlowski wrote:
>>> On 22/07/2022 20:43, Mark Brown wrote:
>>
>>>> ...with a fallback list required by the bindings so the driver actually
>>>> binds.  Note that bindings are currently not in YAML format so there'd
>>>> be even less enforcement of that than normal, and as they're currently
>>>> written the bindings don't require fallback.
>>
>>> Yes, the bindings document should be rephrased but we were living like
>>> that for few years. :)
>>
>> The binding document as it stands only has one compatible, there's no
>> existing problem with it other than the YAML conversion.  If we're
>> adding something new that requires a fallback we should be explicit
>> about that rather than have something that's actively misleading where
>> previously things were clear.  I don't mind if we add the compatible to
>> the driver or document the requirement for the fallback but we should do
>> one of the two.
> 
> is V2 good enough? adding the compatible to the driver and the document?
> Or should we use fallback?
> If fallback is choosen, can you explain how I should do it?

I propose to use fallback. The preferred way is to convert it to DT
schema and then add new device support (so two commits). Other
acceptable way is to rephrase the TXT so it clearly states desired
compatibles - one for old device, two for new devices. There are plenty
of examples in current sources.


Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-binding: spi: npcm-pspi: Add npcm845 compatible

[Krzysztof Kozlowski]

On 26/07/2022 11:30, Tomer Maimon wrote:

>> To answer your question on fallback, just put:
>>
>>     "nuvoton,npcm845-pspi", "nuvoton,npcm750-pspi" for NPCM8XX BMC
> In the document?
> because I don't remember doing so in other documents that I use fallback.

If they also you fallbacks, then the other documents have now incorrect
text.

> I need to do it in ymal files as well?

Depends on what you want to add and what you actually added... Let's be
specific - point to specific commits or files in linux-next. If they
were not applied - give lore.kernel.org link.


Best regards,
Krzysztof

Re: [PATCH v2 2/2] spi: npcm-pspi: Add NPCM845 peripheral SPI support

[Tomer Maimon]

Hi Krzysztof,

Thanks for your explanation.

On Tue, 26 Jul 2022 at 12:47, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 24/07/2022 10:44, Tomer Maimon wrote:
> > Hi Mark and Krzysztof,
> >
> > Thanks for your reply,
> >
> > On Fri, 22 Jul 2022 at 21:57, Mark Brown <broonie@kernel.org> wrote:
> >>
> >> On Fri, Jul 22, 2022 at 08:47:24PM +0200, Krzysztof Kozlowski wrote:
> >>> On 22/07/2022 20:43, Mark Brown wrote:
> >>
> >>>> ...with a fallback list required by the bindings so the driver actually
> >>>> binds.  Note that bindings are currently not in YAML format so there'd
> >>>> be even less enforcement of that than normal, and as they're currently
> >>>> written the bindings don't require fallback.
> >>
> >>> Yes, the bindings document should be rephrased but we were living like
> >>> that for few years. :)
> >>
> >> The binding document as it stands only has one compatible, there's no
> >> existing problem with it other than the YAML conversion.  If we're
> >> adding something new that requires a fallback we should be explicit
> >> about that rather than have something that's actively misleading where
> >> previously things were clear.  I don't mind if we add the compatible to
> >> the driver or document the requirement for the fallback but we should do
> >> one of the two.
> >
> > is V2 good enough? adding the compatible to the driver and the document?
> > Or should we use fallback?
> > If fallback is choosen, can you explain how I should do it?
>
> I propose to use fallback. The preferred way is to convert it to DT
> schema and then add new device support (so two commits). Other
> acceptable way is to rephrase the TXT so it clearly states desired
> compatibles - one for old device, two for new devices. There are plenty
> of examples in current sources.
Appreciate if you could clarify.
in case we use DT-schema, we dont describe the fallback like we doing
in txt document?
I mean that in the yaml file we should describe the NPCM PSPI
compatible property as follow:
  compatible:
    enum:
      - nuvoton,npcm750-pspi
      - nuvoton,npcm845-pspi

If yes, how should the user know that he needs to use fallback incase
is using nuvoton,npcm845-pspi? only from the device tree?
>
>
> Best regards,
> Krzysztof

Best regards,

Tomer

Re: [PATCH v2 2/2] spi: npcm-pspi: Add NPCM845 peripheral SPI support

[Krzysztof Kozlowski]

On 26/07/2022 21:32, Tomer Maimon wrote:
> Hi Krzysztof,
> 
> Thanks for your explanation.
> 
> On Tue, 26 Jul 2022 at 12:47, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 24/07/2022 10:44, Tomer Maimon wrote:
>>> Hi Mark and Krzysztof,
>>>
>>> Thanks for your reply,
>>>
>>> On Fri, 22 Jul 2022 at 21:57, Mark Brown <broonie@kernel.org> wrote:
>>>>
>>>> On Fri, Jul 22, 2022 at 08:47:24PM +0200, Krzysztof Kozlowski wrote:
>>>>> On 22/07/2022 20:43, Mark Brown wrote:
>>>>
>>>>>> ...with a fallback list required by the bindings so the driver actually
>>>>>> binds.  Note that bindings are currently not in YAML format so there'd
>>>>>> be even less enforcement of that than normal, and as they're currently
>>>>>> written the bindings don't require fallback.
>>>>
>>>>> Yes, the bindings document should be rephrased but we were living like
>>>>> that for few years. :)
>>>>
>>>> The binding document as it stands only has one compatible, there's no
>>>> existing problem with it other than the YAML conversion.  If we're
>>>> adding something new that requires a fallback we should be explicit
>>>> about that rather than have something that's actively misleading where
>>>> previously things were clear.  I don't mind if we add the compatible to
>>>> the driver or document the requirement for the fallback but we should do
>>>> one of the two.
>>>
>>> is V2 good enough? adding the compatible to the driver and the document?
>>> Or should we use fallback?
>>> If fallback is choosen, can you explain how I should do it?
>>
>> I propose to use fallback. The preferred way is to convert it to DT
>> schema and then add new device support (so two commits). Other
>> acceptable way is to rephrase the TXT so it clearly states desired
>> compatibles - one for old device, two for new devices. There are plenty
>> of examples in current sources.
> Appreciate if you could clarify.
> in case we use DT-schema, we dont describe the fallback like we doing
> in txt document?
> I mean that in the yaml file we should describe the NPCM PSPI
> compatible property as follow:
>   compatible:
>     enum:
>       - nuvoton,npcm750-pspi
>       - nuvoton,npcm845-pspi
> 
> If yes, how should the user know that he needs to use fallback incase
> is using nuvoton,npcm845-pspi? only from the device tree?
>

If you use DT schema format, you must code it as with fallback (so
items). This is the most popular setup for SoC components so you will
easily find such examples.


Best regards,
Krzysztof

Re: [PATCH v2 0/2] spi: npcm-pspi: add Arbel NPCM8XX support

[Mark Brown]

On Fri, 22 Jul 2022 14:41:34 +0300, Tomer Maimon wrote:
> This patch set adds Arbel NPCM8XX Peripheral SPI (PSPI) support to PSPI NPCM
> driver.
> 
> This patch set was separated from the full duplex patch.
> https://lore.kernel.org/lkml/YtlES7MX6nJr8l+L@sirena.org.uk/
> 
> Addressed comments from:
>  - Mark Brown : https://www.spinics.net/lists/kernel/msg4447178.html
> 
> [...]

Applied to

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

Thanks!

[1/2] dt-binding: spi: npcm-pspi: Add npcm845 compatible
      commit: ac2c55e2260e0ae019119e1b2a52dda138039841
[2/2] spi: npcm-pspi: Add NPCM845 peripheral SPI support
      commit: 6db8595a3e1996654a130760d363912cdd28706d

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