Hi Rob, Thanks for your review. On Tue, 21 Nov 2023 at 15:16, Rob Herring <robh@xxxxxxxxxx> wrote: > > On Mon, Nov 20, 2023 at 09:20:27PM +0000, Peter Griffin wrote: > > Specifying samsung,uart-fifosize in both DT and driver static data is error > > prone and relies on driver probe order and dt aliases to be correct. > > > > Additionally on many Exynos platforms these are (USI) universal serial > > interfaces which can be uart, spi or i2c, so it can change per board. > > > > For google,gs101-uart and exynosautov9-uart make samsung,uart-fifosize a > > required property. For these platforms fifosize now *only* comes from DT. > > > > It is hoped other Exynos platforms will also switch over time. > > Then allow the property on them. Not sure I fully understand your comment. Can you elaborate? Do you mean leave the 'samsung,uart-fifosize' as an optional property like it is currently even for the platforms that now require it to be present to function correctly? I deliberately restricted the yaml change to only require this property for the SoCs that already set the 'samsung,uart-fifosize' dt property. As setting the property and having the driver use what is specified in DT also requires a corresponding driver update (otherwise fifosize gets overwritten by the driver static data, and then becomes dependent on probe order, dt aliases etc). The rationale was drivers 'opt in' and add themselves to the compatibles in this patch as they migrate away from obtaining fifo size from driver static data to obtaining it from DT. > > > > > Signed-off-by: Peter Griffin <peter.griffin@xxxxxxxxxx> > > --- > > .../bindings/serial/samsung_uart.yaml | 17 +++++++++++++++++ > > 1 file changed, 17 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/serial/samsung_uart.yaml b/Documentation/devicetree/bindings/serial/samsung_uart.yaml > > index ccc3626779d9..22a1edadc4fe 100644 > > --- a/Documentation/devicetree/bindings/serial/samsung_uart.yaml > > +++ b/Documentation/devicetree/bindings/serial/samsung_uart.yaml > > @@ -133,6 +133,23 @@ allOf: > > - const: uart > > - const: clk_uart_baud0 > > > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - google,gs101-uart > > + - samsung,exynosautov9-uart > > + then: > > + properties: > > + samsung,uart-fifosize: > > + description: The fifo size supported by the UART channel. > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: [16, 64, 256] > > We already have 'fifo-size' in several drivers. Use that. Please move > its type/description definitions to serial.yaml and make drivers just do > 'fifo-size: true' if they use it. What do you suggest we do for the samsung,uart-fifosize property that is being used upstream? > > > + > > + required: > > + - samsung,uart-fifosize > > A new required property is an ABI break. Please explain why that is okay > in the commit message. > I can update the commit message to make clear there is an ABI break. As mentioned above the platforms where this is now required are either already setting the property or are new in this series. Is that sufficient justification? regards, Peter
