Re: [PATCH v2 08/20] dt-bindings: serial: samsung: Add google-gs101-uart compatible

["Arnd Bergmann" <arnd@xxxxxxxx>]

On Wed, Oct 11, 2023, at 11:42, Greg Kroah-Hartman wrote:
> On Wed, Oct 11, 2023 at 11:30:25AM +0200, Arnd Bergmann wrote:
>> On Wed, Oct 11, 2023, at 10:57, Greg KH wrote:
>> >
>> >> It's not only the IP itself
>> >> that can differ, it's also the integration of the IP into the final
>> >> product that could have an influence on the behavior.
>> >
>> > This is for the Pixel 6, a device that is no longer even shipping.  The
>> > "final product" is long stable, so this should not be an issue.
>> 
>> The driver does have soc specific settings for each compatible
>> string, in this case it looks like it overrides the FIFO size
>> based on driver specific data and the order in which the
>> ports are probed [1]. I don't understand why the driver does
>> this, but my impression is that if we wanted to change it to no
>> longer rely on that data, we'd also need a new compatible
>> string.
>
> As I reviewed that patch already, it is just duplicating an existing
> quirk/device that the driver already supports, so there is no need for
> any "new device type" to be added to that driver, just use the existing
> hardware description in the dt and all should be fine.

The thing is, I suspect that the FIFO size override is actually
wrong for the exynos850 as well, and is almost certainly wrong
for both exynosautov9 and google-gs101:

- The driver overrides an exynos850 compatible uart to use a
  256 byte FIFO on whichever port is probed first, 64 byte
  on the next three ports, and the setting from DT on any
  later ones, falling back to 16 bytes if the DT does not set
  anything.

- exynos850 only actually has three of these ports, not
  four. It does not lists  FIFO size in the dts at all.

- exynosautov9 has a total of 11 ports, each of these
  compatible with both "samsung,exynosautov9-uart" as
  the specific value and "samsung,exynos850-uart" as
  the generic fallback. The DT lists a FIFO size of 256
  bytes for ports 0, 1, and 6, but lists FIFO size 64
  for each of the other ones.

- google-gs101 only lists a single uart in the dts,
  and sets it to a 256 byte FIFO.

- testla-fsd claims to be compatible with exynos4210,
  which also overrides the first two ports in probe
  order to 256 and 64 bytes respectively (like exynos850),
  but it only has two ports.

- artpec8 has a separate compatible string so it overrides
  all ports to 64 bytes.

I don't know why probe order would have anything to do
with this, so most likely these are all the same thing
and should just put a fixed FIFO size into the DT for
each port instance.

      Arnd