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

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(+)

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

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 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

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