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

On Tue, Nov 28, 2023 at 5:02 PM Chen-Yu Tsai [off-list ref] wrote:

On Tue, Nov 28, 2023 at 4:59 PM Chen-Yu Tsai [off-list ref] wrote:

quoted

On Tue, Nov 28, 2023 at 4:30 PM Krzysztof Kozlowski
[off-list ref] wrote:

quoted

On 28/11/2023 08:50, Chen-Yu Tsai wrote:

quoted

On Tue, Nov 28, 2023 at 3:43 PM Krzysztof Kozlowski
[off-list ref] wrote:

quoted

On 28/11/2023 01:58, Andre Przywara wrote:

quoted

+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.

quoted

+     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.

quoted

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.

quoted

quoted

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel