Thread (13 messages) 13 messages, 3 authors, 2025-12-17

Re: [PATCH v2 0/4] s32g: Use a syscon for GPR

From: Frank Li <Frank.li@nxp.com>
Date: 2025-12-16 14:42:25
Also in: imx, linux-arm-kernel, linux-devicetree, lkml

On Tue, Dec 16, 2025 at 10:56:02AM +0300, Dan Carpenter wrote:
On Mon, Dec 15, 2025 at 04:07:48PM -0500, Frank Li wrote:
quoted
On Mon, Dec 15, 2025 at 11:11:03PM +0300, Dan Carpenter wrote:
quoted
On Mon, Dec 15, 2025 at 02:28:43PM -0500, Frank Li wrote:
quoted
On Mon, Dec 15, 2025 at 09:33:54PM +0300, Dan Carpenter wrote:
quoted
On Mon, Dec 15, 2025 at 10:56:49AM -0500, Frank Li wrote:
quoted
On Mon, Dec 15, 2025 at 05:41:43PM +0300, Dan Carpenter wrote:
quoted
The s32g devices have a GPR register region which holds a number of
miscellaneous registers.  Currently only the stmmac/dwmac-s32.c uses
anything from there and we just add a line to the device tree to
access that GMAC_0_CTRL_STS register:

                        reg = <0x4033c000 0x2000>, /* gmac IP */
                              <0x4007c004 0x4>;    /* GMAC_0_CTRL_STS */

We still have to maintain backwards compatibility to this format,
of course, but it would be better to access these through a syscon.
First of all, putting all the registers together is more organized
and shows how the hardware actually is implemented.  Secondly, in
some versions of this chipset those registers can only be accessed
via SCMI, if the registers aren't grouped together each driver will
have to create a whole lot of if then statements to access it via
IOMEM or via SCMI,
Does SCMI work as regmap? syscon look likes simple, but missed abstract
in overall.
The SCMI part of this is pretty complicated and needs discussion.  It
might be that it requires a vendor extension.  Right now, the out of
tree code uses a nvmem vendor extension but that probably won't get
merged upstream.

But in theory, it's fairly simple, you can write a regmap driver and
register it as a syscon and everything that was accessing nxp,phy-sel
accesses the same register but over SCMI.
nxp,phy-sel is not standard API. Driver access raw register value. such
as write 1 to offset 0x100.

After change to SCMI, which may mapped to difference command. Even change
to other SOC, value and offset also need be changed. It is not standilzed
as what you expected.
We're writing to an offset in a syscon.  Right now the device tree
says that the syscon is an MMIO syscon.  But for SCMI devices we
would point the phandle to a custom syscon.  The phandle and the offset
would stay the same, but how the syscon is implemented would change.
Your SCMI syscon driver will convert some private hard code to some
function, such previous example's '1' as SEL_RGMII. It is hard maintained
in long term.
No, there isn't any conversion needed.  It's exactly the same as writing
to the register except it goes through SCMI.
quoted
quoted
quoted
quoted
quoted
You still use regmap by use MMIO. /* GMAC_0_CTRL_STS */

regmap = devm_regmap_init_mmio(dev, sts_offset, &regmap_config);
You can use have an MMIO syscon, or you can create a custom driver
and register it as a syscon using of_syscon_register_regmap().
My means is that it is not necessary to create nxp,phy-sel, especially
there already have <0x4007c004 0x4>;    /* GMAC_0_CTRL_STS */
Right now the out of tree dwmac-s32cc.c driver does something like
this:

    89          if (gmac->use_nvmem) {
    90                  ret = write_nvmem_cell(gmac->dev, "gmac_phy_intf_sel", intf_sel);
    91                  if (ret)
    92                          return ret;
    93          } else {
    94                  writel(intf_sel, gmac->ctrl_sts);
    95          }

Which is quite complicated, but with a syscon, then it's just:

	regmap_write(gmac->sts_regmap, gmac->sts_offset, S32_PHY_INTF_SEL_RGMII);

Even without SCMI, the hardware has all these registers grouped together
it just feels cleaner to group them together in the device tree as well.
Why not implement standard phy interface,
phy_set_mode_ext(PHY_MODE_ETHERNET, RGMII);

For example:  drivers/pci/controller/dwc/pci-imx6.c

In legency platform, it use syscon to set some registers. It becomes mess
when more platform added.  And it becomes hard to convert because avoid
break compatibltiy now.

It doesn't become worse since new platforms switched to use standard
inteface, (phy, reset ...).
This happens below that layer, this is just saying where the registers
are found.  The GMAC_0_CTRL_STS is just one register in the GPR region,
most of the others are unrelated to PHY.
The other register should work as other function's providor with mfd.

Frank
regards,
dan carpenter
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help