2024-01-25 14:52:03

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v2 05/28] spi: dt-bindings: samsung: add samsung,spi-fifosize property

Up to now the SPI alias was used as an index into an array defined in
the SPI driver to determine the SPI FIFO size. Drop the dependency on
the SPI alias and allow the SPI nodes to specify their SPI FIFO size.

Signed-off-by: Tudor Ambarus <[email protected]>
---
Documentation/devicetree/bindings/spi/samsung,spi.yaml | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/samsung,spi.yaml b/Documentation/devicetree/bindings/spi/samsung,spi.yaml
index 2f0a0835ecfb..4ad5b8fe57aa 100644
--- a/Documentation/devicetree/bindings/spi/samsung,spi.yaml
+++ b/Documentation/devicetree/bindings/spi/samsung,spi.yaml
@@ -72,6 +72,11 @@ properties:
reg:
maxItems: 1

+ samsung,spi-fifosize:
+ description: The fifo size supported by the SPI instance.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [64, 256]
+
required:
- compatible
- clocks
--
2.43.0.429.g432eaa2c6b-goog



2024-01-25 16:17:07

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2 05/28] spi: dt-bindings: samsung: add samsung,spi-fifosize property

On Thu, Jan 25, 2024 at 02:49:43PM +0000, Tudor Ambarus wrote:
> Up to now the SPI alias was used as an index into an array defined in
> the SPI driver to determine the SPI FIFO size. Drop the dependency on
> the SPI alias and allow the SPI nodes to specify their SPI FIFO size.

..

> + samsung,spi-fifosize:
> + description: The fifo size supported by the SPI instance.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [64, 256]

Do we have any cases where we'd ever want to vary this independently of
the SoC - this isn't a configurable IP shipped to random integrators?


Attachments:
(No filename) (609.00 B)
signature.asc (499.00 B)
Download all attachments

2024-01-25 16:42:27

by Tudor Ambarus

[permalink] [raw]
Subject: Re: [PATCH v2 05/28] spi: dt-bindings: samsung: add samsung,spi-fifosize property



On 1/25/24 16:16, Mark Brown wrote:
> On Thu, Jan 25, 2024 at 02:49:43PM +0000, Tudor Ambarus wrote:
>> Up to now the SPI alias was used as an index into an array defined in
>> the SPI driver to determine the SPI FIFO size. Drop the dependency on
>> the SPI alias and allow the SPI nodes to specify their SPI FIFO size.
>
> ...
>
>> + samsung,spi-fifosize:
>> + description: The fifo size supported by the SPI instance.
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + enum: [64, 256]
>
> Do we have any cases where we'd ever want to vary this independently of
> the SoC - this isn't a configurable IP shipped to random integrators?

The IP supports FIFO depths from 8 to 256 bytes (in powers of 2 I
guess). The integrator is the one dictating the IP configuration. In
gs101's case all USIxx_USI (which includes SPI, I2C, and UART) are
configured with 64 bytes FIFO depths.

2024-01-25 17:26:51

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2 05/28] spi: dt-bindings: samsung: add samsung,spi-fifosize property

On Thu, Jan 25, 2024 at 04:38:07PM +0000, Tudor Ambarus wrote:
> On 1/25/24 16:16, Mark Brown wrote:

> > Do we have any cases where we'd ever want to vary this independently of
> > the SoC - this isn't a configurable IP shipped to random integrators?

> The IP supports FIFO depths from 8 to 256 bytes (in powers of 2 I
> guess). The integrator is the one dictating the IP configuration. In
> gs101's case all USIxx_USI (which includes SPI, I2C, and UART) are
> configured with 64 bytes FIFO depths.

OK, so just the compatible is enough information then?


Attachments:
(No filename) (569.00 B)
signature.asc (499.00 B)
Download all attachments

2024-01-25 17:31:39

by Tudor Ambarus

[permalink] [raw]
Subject: Re: [PATCH v2 05/28] spi: dt-bindings: samsung: add samsung,spi-fifosize property



On 1/25/24 17:26, Mark Brown wrote:
> On Thu, Jan 25, 2024 at 04:38:07PM +0000, Tudor Ambarus wrote:
>> On 1/25/24 16:16, Mark Brown wrote:
>
>>> Do we have any cases where we'd ever want to vary this independently of
>>> the SoC - this isn't a configurable IP shipped to random integrators?
>
>> The IP supports FIFO depths from 8 to 256 bytes (in powers of 2 I
>> guess). The integrator is the one dictating the IP configuration. In
>> gs101's case all USIxx_USI (which includes SPI, I2C, and UART) are
>> configured with 64 bytes FIFO depths.
>
> OK, so just the compatible is enough information then?

For gs101, yes. All the gs101 SPI instances are configured with 64 bytes
FIFO depths. So instead of specifying the FIFO depth for each SPI node,
we can infer the FIFO depth from the compatible.

2024-01-25 17:46:14

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2 05/28] spi: dt-bindings: samsung: add samsung,spi-fifosize property

On Thu, Jan 25, 2024 at 05:30:53PM +0000, Tudor Ambarus wrote:
> On 1/25/24 17:26, Mark Brown wrote:

> > OK, so just the compatible is enough information then?

> For gs101, yes. All the gs101 SPI instances are configured with 64 bytes
> FIFO depths. So instead of specifying the FIFO depth for each SPI node,
> we can infer the FIFO depth from the compatible.

But this is needed for other SoCs? This change is scattered through a
very large series which does multiple things so it's a bit difficult to
follow what's going on here.


Attachments:
(No filename) (547.00 B)
signature.asc (499.00 B)
Download all attachments

2024-01-25 19:02:22

by Tudor Ambarus

[permalink] [raw]
Subject: Re: [PATCH v2 05/28] spi: dt-bindings: samsung: add samsung,spi-fifosize property



On 1/25/24 17:45, Mark Brown wrote:
> On Thu, Jan 25, 2024 at 05:30:53PM +0000, Tudor Ambarus wrote:
>> On 1/25/24 17:26, Mark Brown wrote:
>
>>> OK, so just the compatible is enough information then?
>
>> For gs101, yes. All the gs101 SPI instances are configured with 64 bytes
>> FIFO depths. So instead of specifying the FIFO depth for each SPI node,
>> we can infer the FIFO depth from the compatible.
>
> But this is needed for other SoCs? This change is scattered through a

There are SoCs that have multiple instances of the SPI IP, and they
configure them with different FIFO depths. See
"samsung,exynosautov9-spi" for example: SPI0, SPI1, and SPI6 are
configured by the SoC to use 256 bytes FIFO depths, while all the other
8 SPI instances use 64 bytes FIFOs. I tried to explain the entire logic
of the series in another reply, see it here:
https://lore.kernel.org/linux-arm-kernel/[email protected]/T/#u

> very large series which does multiple things so it's a bit difficult to
> follow what's going on here.

Sorry, I was afraid that this might happen. I agree, I'll split tomorrow
this patch set in 3 smaller sets:
1/ dumb cleaning patches
2/ heavy lifting cleaning patches
3/ gs101 addition

2024-01-30 22:25:51

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 05/28] spi: dt-bindings: samsung: add samsung,spi-fifosize property

On Thu, Jan 25, 2024 at 07:01:10PM +0000, Tudor Ambarus wrote:
>
>
> On 1/25/24 17:45, Mark Brown wrote:
> > On Thu, Jan 25, 2024 at 05:30:53PM +0000, Tudor Ambarus wrote:
> >> On 1/25/24 17:26, Mark Brown wrote:
> >
> >>> OK, so just the compatible is enough information then?
> >
> >> For gs101, yes. All the gs101 SPI instances are configured with 64 bytes
> >> FIFO depths. So instead of specifying the FIFO depth for each SPI node,
> >> we can infer the FIFO depth from the compatible.
> >
> > But this is needed for other SoCs? This change is scattered through a
>
> There are SoCs that have multiple instances of the SPI IP, and they
> configure them with different FIFO depths. See
> "samsung,exynosautov9-spi" for example: SPI0, SPI1, and SPI6 are
> configured by the SoC to use 256 bytes FIFO depths, while all the other
> 8 SPI instances use 64 bytes FIFOs. I tried to explain the entire logic
> of the series in another reply, see it here:
> https://lore.kernel.org/linux-arm-kernel/[email protected]/T/#u

We have some common properties for fifo size. In fact, there was just a
discussion recently on Samsung UART (Is that the same block?) about
this. So if you do use a property here, use a common one.

Rob

2024-02-05 10:11:16

by Tudor Ambarus

[permalink] [raw]
Subject: Re: [PATCH v2 05/28] spi: dt-bindings: samsung: add samsung,spi-fifosize property

Hi, Rob,

On 31.01.2024 00:25, Rob Herring wrote:
> On Thu, Jan 25, 2024 at 07:01:10PM +0000, Tudor Ambarus wrote:
>>
>>
>> On 1/25/24 17:45, Mark Brown wrote:
>>> On Thu, Jan 25, 2024 at 05:30:53PM +0000, Tudor Ambarus wrote:
>>>> On 1/25/24 17:26, Mark Brown wrote:
>>>
>>>>> OK, so just the compatible is enough information then?
>>>
>>>> For gs101, yes. All the gs101 SPI instances are configured with 64 bytes
>>>> FIFO depths. So instead of specifying the FIFO depth for each SPI node,
>>>> we can infer the FIFO depth from the compatible.
>>>
>>> But this is needed for other SoCs? This change is scattered through a
>>
>> There are SoCs that have multiple instances of the SPI IP, and they
>> configure them with different FIFO depths. See
>> "samsung,exynosautov9-spi" for example: SPI0, SPI1, and SPI6 are
>> configured by the SoC to use 256 bytes FIFO depths, while all the other
>> 8 SPI instances use 64 bytes FIFOs. I tried to explain the entire logic
>> of the series in another reply, see it here:
>> https://lore.kernel.org/linux-arm-kernel/[email protected]/T/#u
>
> We have some common properties for fifo size. In fact, there was just a
> discussion recently on Samsung UART (Is that the same block?) about

It is the same block, I'll take a look.

> this. So if you do use a property here, use a common one.

Will do. Thanks, Rob!
Cheers,
ta