On Mon, 20 Aug 2018 16:41:09 +0800 Chen-Yu Tsai <wens@xxxxxxxx> wrote: > On Mon, Aug 20, 2018 at 3:42 PM, Emmanuel Vadot <manu@xxxxxxxxxxxxxxxx> wrote: > > > > Hello Chen-Yu, > > > > On Thu, 14 Jun 2018 23:35:44 +0800 > > Chen-Yu Tsai <wens@xxxxxxxx> wrote: > > > >> Hi, > >> > >> This series is the remaining A64 syscon changes from the R40 DWMAC > >> series. The series aligns how the A64 system control exports a regmap > >> for the sun8i DWMAC driver to access with what we've done for the R40. > >> > >> Originally the A64 used the generic syscon for this bit of hardware. > >> But this block also contains mapping bits for the onboard SRAM, used > >> by various peripherals, and other vendor specific bits we may use in > >> the future. It is by no means generic. And we already have a device > >> tree binding and driver for the SRAM part. > >> > >> The first patch make the SRAM control device export a regmap, exposing > >> a single EMAC control register, for the DWMAC driver to consume. > >> > >> The second and third patches rename the A64 compatible string to read > >> "system control", which is what the block is named in the user manual. > >> > >> The last patch fixes up the device node, and also adds the lone mappable > >> SRAM block, which is needed by the Display Engine. > >> > >> Changes since v2: > >> > >> - changed the compatible string from "*-sram-controller" to > >> "*-system-control" > >> > >> > >> ChenYu > >> > >> Chen-Yu Tsai (2): > >> dt-bindings: sram: Rename A64 SRAM controller compatible > >> soc: sunxi: sram: Add updated compatible string for A64 system control > >> > >> Icenowy Zheng (2): > >> soc: sunxi: export a regmap for EMAC clock reg on A64 > >> arm64: dts: allwinner: a64: add SRAM controller device tree node > >> > >> .../devicetree/bindings/sram/sunxi-sram.txt | 3 +- > >> arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 19 +++++- > >> drivers/soc/sunxi/sunxi_sram.c | 61 ++++++++++++++++++- > >> 3 files changed, 78 insertions(+), 5 deletions(-) > >> > >> -- > >> 2.17.1 > > > > I wish to have seen this serie before as it have some inconsistencies. > > > > In patch 2 you renamed allwinner,sun50i-a64-sram-controller to > > allwinner,sun50i-a64-system-control but the former was never used in > > the DTS, the compatible used was allwinner,sun50i-a64-system-controller. > > You also say that you've never seen use of it. How can you make that > > claim ? There is a lot of downstream users of DTS now (FreeBSD, NetBSD, > > OpenBSD and even RiscOS and Haiku iirc), it's not just Linux. > > The original "a64-sram-controller" was never used in any upstream device > tree, be it Linux or U-boot. Since the projects you listed all derive > their device trees from U-boot, with maybe some extra devices on top, > they would either have had the "system-controller" version, or the even > earlier version in U-boot where the emac node just lists the syscon > address range as its own. > > U-boot has the latter "system-controller" because it was copied from Linux. > The original use-case was a SoC-specific compatible string followed by the > very generic "syscon". The latter "syscon" binding is what we actually > support in Linux and U-boot. Same goes for NetBSD and OpenBSD. > > IMHO FreeBSD could also move to a generic syscon API, instead of > "system-controller" for sunxi and "grf" for rockchip. We decided to always implement a specific driver if the node have another compatible than syscon (I don't see the point of having a syscon compatible plus another one) > You can continue to support old device trees and the unlisted compatible > through the generic "syscon" fallback. For the updated device tree you will > have to support the new compatible, along with supporting the SRAM mappings. > The SRAM mappings are why the "syscon" compatible was removed. It just > doesn't fit the semantics described by the "syscon" binding. I understand the removal of syscon since the sram addition, I just don't understand the strict renaming and why adding a new one couldn't work. > > Also this compatible is currently the one used in the u-boot dts, > > which mean that users of the embedded DTB use or can use it (which is > > the default for EFI users of U-Boot). > > As mentioned above, the old device tree uses the syscon binding, which > you can continue to support. The new binding is add support for the SRAM > mapping system. In addition, you probably don't want two device drivers > supporting the same compatible string. On Linux it's even worse, because > the "syscon" driver sidesteps the device model and isn't an actual device > driver. > > Regards > ChenYu > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- Emmanuel Vadot <manu@xxxxxxxxxxxxxxxx> <manu@xxxxxxxxxxx>