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

[Chen-Yu Tsai]

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.


ChenYu

> 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