[PATCH v3 4/6] thermal: sun8i: add syscon register access code

[Chen-Yu Tsai]

On Tue, Nov 28, 2023 at 5:02 PM Chen-Yu Tsai <wens at csie.org> wrote:
>
> On Tue, Nov 28, 2023 at 4:59 PM Chen-Yu Tsai <wens at csie.org> wrote:
> >
> > On Tue, Nov 28, 2023 at 4:30 PM Krzysztof Kozlowski
> > <krzysztof.kozlowski at linaro.org> wrote:
> > >
> > > On 28/11/2023 08:50, Chen-Yu Tsai wrote:
> > > > On Tue, Nov 28, 2023 at 3:43 PM Krzysztof Kozlowski
> > > > <krzysztof.kozlowski at linaro.org> 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