LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Maxime Ripard <maxime.ripard@bootlin.com>
To: Chen-Yu Tsai <wens@csie.org>
Cc: Icenowy Zheng <icenowy@aosc.io>, Rob Herring <robh+dt@kernel.org>,
	Giuseppe Cavallaro <peppe.cavallaro@st.com>,
	Corentin Labbe <clabbe.montjoie@gmail.com>,
	netdev <netdev@vger.kernel.org>,
	devicetree <devicetree@vger.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linux-sunxi <linux-sunxi@googlegroups.com>
Subject: Re: [linux-sunxi] Re: [PATCH 3/5] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device
Date: Mon, 16 Apr 2018 16:31:30 +0200	[thread overview]
Message-ID: <20180416143130.tls6xtcq3hsv7u7f@flea> (raw)
In-Reply-To: <CAGb2v66Qi2D1qgF3-iVJFAiznMUzzen0nHM5vsPe7Cyi6QTYpw@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 3467 bytes --]

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@aosc.io> wrote:
> > 于 2018年4月12日 GMT+08:00 下午10:56:28, Maxime Ripard <maxime.ripard@bootlin.com> 写到:
> >>On Wed, Apr 11, 2018 at 10:16:39PM +0800, Icenowy Zheng wrote:
> >>> From: Chen-Yu Tsai <wens@csie.org>
> >>>
> >>> 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@csie.org>
> >>> [Icenowy: change to use regmaps with single register, change commit
> >>>  message]
> >>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> >>> ---
> >>>  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.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2018-04-16 14:31 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-11 14:16 [PATCH 0/5] Add support in dwmac-sun8i for accessing EMAC clock Icenowy Zheng
2018-04-11 14:16 ` [PATCH 1/5] dt-bindings: allow dwmac-sun8i to use other devices' exported regmap Icenowy Zheng
2018-04-11 14:36   ` [linux-sunxi] " Brüns, Stefan
2018-04-16 18:47   ` Rob Herring
2018-04-16 23:17     ` Icenowy Zheng
2018-04-28 13:42       ` [linux-sunxi] " Chen-Yu Tsai
2018-04-11 14:16 ` [PATCH 2/5] net: stmmac: dwmac-sun8i: Use regmap_field for syscon register access Icenowy Zheng
2018-04-11 14:16 ` [PATCH 3/5] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device Icenowy Zheng
2018-04-12 14:56   ` Maxime Ripard
2018-04-12 15:11     ` Icenowy Zheng
2018-04-12 15:23       ` [linux-sunxi] " Chen-Yu Tsai
2018-04-16 14:31         ` Maxime Ripard [this message]
2018-04-16 14:34           ` Icenowy Zheng
2018-04-16 14:51           ` Chen-Yu Tsai
2018-04-17 11:52             ` Maxime Ripard
2018-04-17 11:59               ` Chen-Yu Tsai
2018-04-17 12:06                 ` Icenowy Zheng
2018-04-13  6:03   ` [RFC PATCH] net: stmmac: dwmac-sun8i: single_reg_field can be static kbuild test robot
2018-04-13  6:03   ` [PATCH 3/5] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device kbuild test robot
2018-04-11 14:16 ` [PATCH 4/5] drivers: soc: sunxi: export a regmap for EMAC clock reg on A64 Icenowy Zheng
2018-04-11 14:16 ` [PATCH 5/5] arm64: allwinner: a64: add SRAM controller device tree node Icenowy Zheng
2018-04-12 14:58   ` Maxime Ripard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180416143130.tls6xtcq3hsv7u7f@flea \
    --to=maxime.ripard@bootlin.com \
    --cc=clabbe.montjoie@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=icenowy@aosc.io \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sunxi@googlegroups.com \
    --cc=netdev@vger.kernel.org \
    --cc=peppe.cavallaro@st.com \
    --cc=robh+dt@kernel.org \
    --cc=wens@csie.org \
    --subject='Re: [linux-sunxi] Re: [PATCH 3/5] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).