On Tue, Nov 28, 2023 at 5:02 PM Chen-Yu Tsai <wens@xxxxxxxx> wrote: > > On Tue, Nov 28, 2023 at 4:59 PM Chen-Yu Tsai <wens@xxxxxxxx> wrote: > > > > On Tue, Nov 28, 2023 at 4:30 PM Krzysztof Kozlowski > > <krzysztof.kozlowski@xxxxxxxxxx> wrote: > > > > > > On 28/11/2023 08:50, Chen-Yu Tsai wrote: > > > > On Tue, Nov 28, 2023 at 3:43 PM Krzysztof Kozlowski > > > > <krzysztof.kozlowski@xxxxxxxxxx> wrote: > > > >> > > > >> On 28/11/2023 01:58, Andre Przywara wrote: > > > >>> > > > >>> +static struct regmap *sun8i_ths_get_syscon_regmap(struct device_node *node) > > > >>> +{ > > > >>> + struct device_node *syscon_node; > > > >>> + struct platform_device *syscon_pdev; > > > >>> + struct regmap *regmap = NULL; > > > >>> + > > > >>> + syscon_node = of_parse_phandle(node, "syscon", 0); > > > >> > > > >> Nope. For the 100th time, this cannot be generic. > > > >> > > > >>> + if (!syscon_node) > > > >>> + return ERR_PTR(-ENODEV); > > > >>> + > > > >>> + syscon_pdev = of_find_device_by_node(syscon_node); > > > >>> + if (!syscon_pdev) { > > > >>> + /* platform device might not be probed yet */ > > > >>> + regmap = ERR_PTR(-EPROBE_DEFER); > > > >>> + goto out_put_node; > > > >>> + } > > > >>> + > > > >>> + /* If no regmap is found then the other device driver is at fault */ > > > >>> + regmap = dev_get_regmap(&syscon_pdev->dev, NULL); > > > >>> + if (!regmap) > > > >>> + regmap = ERR_PTR(-EINVAL); > > > >> > > > >> Aren't you open-coding existing API to get regmap from syscon? > > > > > > > > Not really. This is to get a regmap exported by the device. Syscon's regmap > > > > is not tied to the device at all. > > > > > > I am talking about open-coding existing API. Look at syscon.h. > > > > The underlying implementation is different. > > > > syscon maintains its own mapping of device nodes to regmaps, and creates > > regmaps when none exist. The regmap is not tied to any struct device. > > syscon basically exists outside of the driver model and one has no control > > over what is exposed because it is meant for blocks that are a collection > > of random stuff. > > My bad. I failed to realize there is a platform device driver for syscon, > in addition to the existing "no struct device" implementation. Actually that doesn't do anything on DT platforms as of commit bdb0066df96e ("mfd: syscon: Decouple syscon interface from platform devices"). All the regmaps are, as I previously stated, not tied to any struct device. > > Here the provider device registers the (limited) regmap it wants to provide, > > tying the new regmap to itself. The consumer gets it via the dev_get_regmap() > > call. The provider has a main function and isn't exposing that part of its > > register map to the outside; only the random bits that were stuffed in are. > > > > > > With this scheme a device to could export just enough registers for the > > > > consumer to use, instead of the whole address range. > > > > > > > > We do this in the R40 clock controller as well, which has some bits that > > > > tweak the ethernet controllers RGMII delay... > > > > > > Not related. > > > > Related as in that is possibly what this code was based on, commit > > 49a06cae6e7c ("net: stmmac: dwmac-sun8i: Allow getting syscon regmap > > from external device"). > > > > > > ChenYu