diff mbox series

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

Message ID 20240125145007.748295-6-tudor.ambarus@linaro.org
State New
Headers show
Series spi: s3c64xx: winter cleanup and gs101 support | expand

Commit Message

Tudor Ambarus Jan. 25, 2024, 2:49 p.m. UTC
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 <tudor.ambarus@linaro.org>
---
 Documentation/devicetree/bindings/spi/samsung,spi.yaml | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Tudor Ambarus Jan. 25, 2024, 4:38 p.m. UTC | #1
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.
Mark Brown Jan. 25, 2024, 5:26 p.m. UTC | #2
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?
Mark Brown Jan. 25, 2024, 5:45 p.m. UTC | #3
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.
Rob Herring (Arm) Jan. 30, 2024, 10:25 p.m. UTC | #4
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/40ba9481-4aea-4a72-87bd-c2db319be069@linaro.org/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
Tudor Ambarus Feb. 5, 2024, 9:42 a.m. UTC | #5
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/40ba9481-4aea-4a72-87bd-c2db319be069@linaro.org/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
diff mbox series

Patch

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