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/[email protected]/
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
Add a compatible string for Nuvoton BMC NPCM845 PSPI.
Signed-off-by: Tomer Maimon <[email protected]>
---
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
Add Nuvoton BMC NPCM845 NPCM Peripheral SPI (PSPI) support.
Signed-off-by: Tomer Maimon <[email protected]>
---
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
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.
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
On 22/07/2022 13:41, Tomer Maimon wrote:
> Add Nuvoton BMC NPCM845 NPCM Peripheral SPI (PSPI) support.
>
> Signed-off-by: Tomer Maimon <[email protected]>
> ---
> 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
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.
Hi Mark and Krzysztof,
Thanks for your reply,
On Fri, 22 Jul 2022 at 21:57, Mark Brown <[email protected]> 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
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 <[email protected]>
> ---
> 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
>
>
Hi Rob,
Thanks for your comments.
On Tue, 26 Jul 2022 at 01:56, Rob Herring <[email protected]> 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 <[email protected]>
> > ---
> > 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
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
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 <[email protected]> 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
Hi Krzysztof,
Thanks for your explanation.
On Tue, 26 Jul 2022 at 12:47, Krzysztof Kozlowski
<[email protected]> 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 <[email protected]> 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
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
> <[email protected]> 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 <[email protected]> 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
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/[email protected]/
>
> 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