On Tue, Apr 17, 2018 at 7:52 PM, Maxime Ripard <maxime.ripard@xxxxxxxxxxx> wrote: > On Mon, Apr 16, 2018 at 10:51:55PM +0800, Chen-Yu Tsai wrote: >> On Mon, Apr 16, 2018 at 10:31 PM, Maxime Ripard >> <maxime.ripard@xxxxxxxxxxx> wrote: >> > On Thu, Apr 12, 2018 at 11:23:30PM +0800, Chen-Yu Tsai wrote: >> >> On Thu, Apr 12, 2018 at 11:11 PM, Icenowy Zheng <icenowy@xxxxxxx> wrote: >> >> > 于 2018年4月12日 GMT+08:00 下午10:56:28, Maxime Ripard <maxime.ripard@xxxxxxxxxxx> 写到: >> >> >>On Wed, Apr 11, 2018 at 10:16:39PM +0800, Icenowy Zheng wrote: >> >> >>> From: Chen-Yu Tsai <wens@xxxxxxxx> >> >> >>> >> >> >>> On the Allwinner R40 SoC, the "GMAC clock" register is in the CCU >> >> >>> address space; on the A64 SoC this register is in the SRAM controller >> >> >>> address space, and with a different offset. >> >> >>> >> >> >>> To access the register from another device and hide the internal >> >> >>> difference between the device, let it register a regmap named >> >> >>> "emac-clock". We can then get the device from the phandle, and >> >> >>> retrieve the regmap with dev_get_regmap(); in this situation the >> >> >>> regmap_field will be set up to access the only register in the >> >> >>regmap. >> >> >>> >> >> >>> Signed-off-by: Chen-Yu Tsai <wens@xxxxxxxx> >> >> >>> [Icenowy: change to use regmaps with single register, change commit >> >> >>> message] >> >> >>> Signed-off-by: Icenowy Zheng <icenowy@xxxxxxx> >> >> >>> --- >> >> >>> drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 48 >> >> >>++++++++++++++++++++++- >> >> >>> 1 file changed, 46 insertions(+), 2 deletions(-) >> >> >>> >> >> >>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >> >> >>b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >> >> >>> index 1037f6c78bca..b61210c0d415 100644 >> >> >>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >> >> >>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >> >> >>> @@ -85,6 +85,13 @@ const struct reg_field old_syscon_reg_field = { >> >> >>> .msb = 31, >> >> >>> }; >> >> >>> >> >> >>> +/* Specially exported regmap which contains only EMAC register */ >> >> >>> +const struct reg_field single_reg_field = { >> >> >>> + .reg = 0, >> >> >>> + .lsb = 0, >> >> >>> + .msb = 31, >> >> >>> +}; >> >> >>> + >> >> >> >> >> >>I'm not sure this would be wise. If we ever need some other register >> >> >>exported through the regmap, will have to change all the calling sites >> >> >>everywhere in the kernel, which will be a pain and will break >> >> >>bisectability. >> >> > >> >> > In this situation the register can be exported as another >> >> > regmap. Currently the code will access a regmap with name >> >> > "emac-clock" for this register. >> >> > >> >> >> >> >> >>Chen-Yu's (or was it yours?) initial solution with a custom writeable >> >> >>hook only allowing a single register seemed like a better one. >> >> > >> >> > But I remember you mentioned that you want it to hide the >> >> > difference inside the device. >> >> >> >> The idea is that a device can export multiple regmaps. This one, >> >> the one named "gmac" (in my soon to come v2) or "emac-clock" here, >> >> is but one of many possible regmaps, and it only exports the register >> >> needed by the GMAC/EMAC. >> > >> > I'm not sure this would be wise either. There's a single register map, >> > and as far as I know we don't have a binding to express this in the >> > DT. This means that the customer and provider would have to use the >> > same name, but without anything actually enforcing it aside from >> > "someone in the community knows it". >> > >> > This is not a really good design, and I was actually preferring your >> > first option. We shouldn't rely on any undocumented rule. This will be >> > easy to break and hard to maintain. >> >> So, one regmap per device covering the whole register range, and the >> consumer knows which register to poke by looking at its own compatible. >> >> That sound right? > > Yep. And ideally, sending a single serie for both the A64 and the R40 > cases, in order to provide the big picture. OK. I'll incorporate Icenowy's stuff into my series. ChenYu -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html