| Message ID | 20240502143410.12629-3-prajna.rajendrakumar@microchip.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for GPIO based CS | expand |
On 02/05/2024 16:34, Prajna Rajendra Kumar wrote: > The PolarFire SoC SPI controller supports multiple chip selects,but in > the MSS, only one CS line is physically wired. To reflect this hardware > limitation in the device tree, the binding enforces that the 'num-cs' > property defaults to 1 and cannot exceed 1 unless additional > chip select lines are explicitly defined using GPIO descriptors. > You marked it as Fix for bug, but I don't understand where the bug is. Do you describe above the issue or the solution? > + - if: > + properties: > + compatible: > + contains: > + const: microchip,mpfs-spi > + not: > + required: > + - cs-gpios I don't understand what you are expressing here. Did you actually validate it that it achieves exactly what you want? > + then: > + properties: > + num-cs: > + default: 1 > + maximum: 1 > + > unevaluatedProperties: false > > examples: Best regards, Krzysztof
On Thu, May 02, 2024 at 03:34:09PM +0100, Prajna Rajendra Kumar wrote: > The PolarFire SoC SPI controller supports multiple chip selects,but in > the MSS, only one CS line is physically wired. To reflect this hardware > limitation in the device tree, the binding enforces that the 'num-cs' > property defaults to 1 and cannot exceed 1 unless additional > chip select lines are explicitly defined using GPIO descriptors. > > Fixes: 2da187304e55 ("spi: add bindings for microchip mpfs spi") > Signed-off-by: Prajna Rajendra Kumar <prajna.rajendrakumar@microchip.com> > --- > .../bindings/spi/microchip,mpfs-spi.yaml | 19 ++++++++++++++++--- > 1 file changed, 16 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml b/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml > index 74a817cc7d94..19951951fdd6 100644 > --- a/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml > +++ b/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml > @@ -13,9 +13,6 @@ description: > maintainers: > - Conor Dooley <conor.dooley@microchip.com> > > -allOf: > - - $ref: spi-controller.yaml# > - > properties: > compatible: > oneOf: > @@ -43,6 +40,22 @@ required: > - interrupts > - clocks > > +allOf: > + - $ref: spi-controller.yaml# > + - if: > + properties: > + compatible: > + contains: > + const: microchip,mpfs-spi > + not: > + required: > + - cs-gpios > + then: > + properties: > + num-cs: > + default: 1 > + maximum: 1 I think this isn't quite right actually. The default for num-cs should be 1, regardless of whether cs-gpios are used or not, only the maximum is affected by using GPIOs for chip select. > + > unevaluatedProperties: false > > examples: > -- > 2.25.1 >
On Thu, 2024-05-02 at 16:53 +0200, Krzysztof Kozlowski wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > > On 02/05/2024 16:34, Prajna Rajendra Kumar wrote: > > The PolarFire SoC SPI controller supports multiple chip selects,but > > in > > the MSS, only one CS line is physically wired. To reflect this > > hardware > > limitation in the device tree, the binding enforces that the 'num- > > cs' > > property defaults to 1 and cannot exceed 1 unless additional > > chip select lines are explicitly defined using GPIO descriptors. > > > Hi, > You marked it as Fix for bug, but I don't understand where the bug > is. The bug was that the PolarFire SoC SPI "hard" controller supports eight chip selects, but only one chip select is connected and can be used. This was not reflected in the device tree because default num-cs was never set. Hence, it's marked as a fix. > Do you describe above the issue or the solution? It describes both the issue and the solution, but I will revise the commit message as Conor Dooley suggested. > > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: microchip,mpfs-spi > > + not: > > + required: > > + - cs-gpios > > I don't understand what you are expressing here. Did you actually > validate it that it achieves exactly what you want? Since the controller supports only one chip select, the num-cs should default to 1 and cannot exceed 1 unless GPIOs are used as chip selects. > > + then: > > + properties: > > + num-cs: > > + default: 1 > > + maximum: 1 > > + > > unevaluatedProperties: false > > > > examples: > > Best regards, > Krzysztof > Best regards, Prajna
On 03/05/2024 14:54, Prajna.Rajendrakumar@microchip.com wrote: >> >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + const: microchip,mpfs-spi >>> + not: >>> + required: >>> + - cs-gpios >> >> I don't understand what you are expressing here. Did you actually >> validate it that it achieves exactly what you want? > > Since the controller supports only one chip select, the num-cs should > default to 1 and cannot exceed 1 unless GPIOs are used as chip selects. That's not really the answer... or you want to say you did not test it? Best regards, Krzysztof
On Fri, May 03, 2024 at 04:46:13PM +0200, Krzysztof Kozlowski wrote: > On 03/05/2024 14:54, Prajna.Rajendrakumar@microchip.com wrote: > >> > >>> + - if: > >>> + properties: > >>> + compatible: > >>> + contains: > >>> + const: microchip,mpfs-spi > >>> + not: > >>> + required: > >>> + - cs-gpios > >> > >> I don't understand what you are expressing here. Did you actually > >> validate it that it achieves exactly what you want? > > > > Since the controller supports only one chip select, the num-cs should > > default to 1 and cannot exceed 1 unless GPIOs are used as chip selects. > > That's not really the answer... or you want to say you did not test it? She told me she did, and even if she hadn't, I did, despite what my email earlier today might suggest! Cheers, Conor.
On Fri, 2024-05-03 at 16:46 +0200, Krzysztof Kozlowski wrote: > [Some people who received this message don't often get email from > krzk@kernel.org. Learn why this is important at > https://aka.ms/LearnAboutSenderIdentification ] > > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > > On 03/05/2024 14:54, Prajna.Rajendrakumar@microchip.com wrote: > > > > + - if: > > > > + properties: > > > > + compatible: > > > > + contains: > > > > + const: microchip,mpfs-spi > > > > + not: > > > > + required: > > > > + - cs-gpios > > > > > > I don't understand what you are expressing here. Did you actually > > > validate it that it achieves exactly what you want? > > > > Since the controller supports only one chip select, the num-cs > > should > > default to 1 and cannot exceed 1 unless GPIOs are used as chip > > selects. > > That's not really the answer... or you want to say you did not test > it? > I overlooked mentioning this in my previous reply. It's been thoroughly validated. > Best regards, > Krzysztof > Best regards, Prajna
diff --git a/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml b/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml index 74a817cc7d94..19951951fdd6 100644 --- a/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml +++ b/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml @@ -13,9 +13,6 @@ description: maintainers: - Conor Dooley <conor.dooley@microchip.com> -allOf: - - $ref: spi-controller.yaml# - properties: compatible: oneOf: @@ -43,6 +40,22 @@ required: - interrupts - clocks +allOf: + - $ref: spi-controller.yaml# + - if: + properties: + compatible: + contains: + const: microchip,mpfs-spi + not: + required: + - cs-gpios + then: + properties: + num-cs: + default: 1 + maximum: 1 + unevaluatedProperties: false examples:
The PolarFire SoC SPI controller supports multiple chip selects,but in the MSS, only one CS line is physically wired. To reflect this hardware limitation in the device tree, the binding enforces that the 'num-cs' property defaults to 1 and cannot exceed 1 unless additional chip select lines are explicitly defined using GPIO descriptors. Fixes: 2da187304e55 ("spi: add bindings for microchip mpfs spi") Signed-off-by: Prajna Rajendra Kumar <prajna.rajendrakumar@microchip.com> --- .../bindings/spi/microchip,mpfs-spi.yaml | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)