On Fri, Mar 22, 2024 at 12:26:10PM -0400, Sean Anderson wrote: > On 3/22/24 03:01, Krzysztof Kozlowski wrote: > > On 21/03/2024 17:21, Sean Anderson wrote: > >> On 3/19/24 13:55, Conor Dooley wrote: > >>> On Mon, Mar 18, 2024 at 11:48:06AM -0400, Sean Anderson wrote: > >>>> On 3/18/24 11:40, Conor Dooley wrote: > >>>>> On Mon, Mar 18, 2024 at 11:08:00AM -0400, Sean Anderson wrote: > >>>>>> On 3/17/24 11:10, Conor Dooley wrote: > >>>>> > >>>>>>> Additionally, should > >>>>>>> they fall back to t1023-sfp? I see that there's already some dts files > >>>>>>> with these compatibles in them but seemingly no driver support as there > >>>>>>> is for the t1023-sfp. > >>>>>> > >>>>>> I checked the reference manuals for these processors, and all of them use TA 2.0. > >>>>> > >>>>> Sounds like a fallback is suitable then, although that will require > >>>>> updating the various dts files. > >>>> > >>>> Yes, a fallback (like what is done for the T-series) would be suitable, > >>>> but given that these devicetrees have been in-tree for eight years I > >>>> think it would be preferable to support the existing bindings for > >>>> compatibility purposes. > >>> > >>> Just cos stuff snuck into the tree in dts files doesn't make it right > >>> though, I'd rather the bindings were done correctly. I don't care if you > >>> want to support all of the compatibles in the driver so that it works > >>> with the existing devicetrees though, as long as you mention the > >>> rationale in the commit message. > >> > >> It doesn't really matter what the schema has as long as the driver supports > >> existing device trees. > > > > We do not talk about driver now but bindings. You add new compatibles on > > a basis that they were already used. This cannot bypass regular review > > comments, so if during regular review process we would require > > fallbacks, then you are expected to listen to review also when > > documenting existing compatibles. Otherwise everyone would prefer to > > snuck in incorrect code and later document it "it was there!". > > To be clear, the existing nodes look like > > sfp: sfp@e8000 { > compatible = "fsl,t1040-sfp"; > reg = <0xe8000 0x1000>; > }; > > which is perfectly serviceable for read-only use (as the clock is only > necessary for writing). As these devices are effectively identical, the > compatible could also look like what the P-series has: I'd rather you just picked the oldest device and used that as the fallback, rather than introducing an "<blah>-sfp-2.0" compatible. The 2.1 and 3.0 ones don't have anything like that. > sfp: sfp@e8000 { > compatible = "fsl,p2041-sfp", "fsl,qoriq-sfp-1.0"; > reg = <0xe8000 0x1000>; > }; None of the seem to be documented either btw: rg fsl,qoriq-sfp-1.0 arch/powerpc/boot/dts/fsl/p5020si-post.dtsi 371: compatible = "fsl,p5020-sfp", "fsl,qoriq-sfp-1.0"; arch/powerpc/boot/dts/fsl/p2041si-post.dtsi 339: compatible = "fsl,p2041-sfp", "fsl,qoriq-sfp-1.0"; arch/powerpc/boot/dts/fsl/p4080si-post.dtsi 386: compatible = "fsl,p4080-sfp", "fsl,qoriq-sfp-1.0"; arch/powerpc/boot/dts/fsl/p3041si-post.dtsi 366: compatible = "fsl,p3041-sfp", "fsl,qoriq-sfp-1.0"; arch/powerpc/boot/dts/fsl/p5040si-post.dtsi 331: compatible = "fsl,p5040-sfp", "fsl,qoriq-sfp-1.0"; > > but in either case, it is desirable for the driver to match based on the > more-specific compatible (as well as the less-specific compatible) as we > already have enough information from the more-specific compatible to > select the correct implementation. > > --Sean
Attachment:
signature.asc
Description: PGP signature