Hi Arnd, Thanks for your feedback. On Wed, 11 Oct 2023 at 11:19, Arnd Bergmann <arnd@xxxxxxxx> wrote: > > 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. I agree, I just looked again at gs101 and in total we can have 19 uarts on this SoC. 3 of them are 256byte rx/tx fifo and the other 16 have 64byte tx/rx fifo size, but this is a SoC design choice and has nothing to do with probe order. I will update in v3 to get fifo size from DT. regards, Peter.
