Hi Simon

-----Original Message----- From: sjg@google.com sjg@google.com On Behalf Of Simon Glass Sent: mardi 10 juillet 2018 22:50

Hi Patrick,

On 9 July 2018 at 06:59, Patrick Delaunay patrick.delaunay@st.com wrote:

...

Use new API syscon_node_to_regmap in sysreset_syscon driver for compatible "syscon-reboot"; that's avoid the need of explicit syscon binding for "regmap" handle.

Signed-off-by: Patrick Delaunay patrick.delaunay@st.com

drivers/sysreset/sysreset_syscon.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/sysreset/sysreset_syscon.c b/drivers/sysreset/sysreset_syscon.c index f19e80e..3450640 100644 --- a/drivers/sysreset/sysreset_syscon.c +++ b/drivers/sysreset/sysreset_syscon.c @@ -35,18 +35,20 @@ static struct sysreset_ops syscon_reboot_ops = {

int syscon_reboot_probe(struct udevice *dev) {

•   struct udevice *syscon;
    struct syscon_reboot_priv *priv = dev_get_priv(dev);
    int err;
  

•   u32 phandle;
  

•   ofnode node;
  
  

•   err = uclass_get_device_by_phandle(UCLASS_SYSCON, dev,
  

•                                      "regmap", &syscon);
  

•   if (err) {
  

•           pr_err("unable to find syscon device\n");
  

•   err = ofnode_read_u32(dev_ofnode(dev), "regmap", &phandle);
  

•   if (err)
            return err;
  

•   }
  
  

•   priv->regmap = syscon_get_regmap(syscon);
  

•   node = ofnode_get_by_phandle(phandle);
  

•   if (!ofnode_valid(node))
  

•           return -EINVAL;
  

•   priv->regmap = syscon_node_to_regmap(node);
    if (!priv->regmap) {
            pr_err("unable to find regmap\n");
            return -ENODEV;
  

Aren't you just re-implementing uclass_get_device_by_phandle() here?

Not exactly, because if I use the the function uclass_get_device_by_phandle, the device need to be bounded to syscon UCLASS.

And it is not needed with my proposal as I use the new function syscon_node_to_regmap which not require explicite syscon binding.

I have the problem in stm32mp1 platform with the DT : (./arch/arm/dts/stm32mp157c.dtsi)

rcc: rcc@50000000 { compatible = "st,stm32mp1-rcc", "syscon"; #clock-cells = <1>; #reset-cells = <1>; reg = <0x50000000 0x1000>; st,pwr = <&pwr>; interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 145 IRQ_TYPE_LEVEL_HIGH>; };

rcc_reboot: rcc-reboot@50000000 { compatible = "syscon-reboot"; regmap = <&rcc>; offset = <0x404>; mask = <0x1>; };

1/ rcc is binded to drivers/misc/stm32_rcc.c with compatible "st,stm32mp1-rcc" =>3 roles + Clock provider => bound with stm32mp1_clk driver + Reset provide => bound with "stm32_rcc_reset" + sysreset provider with syscon.... not bound by default

2/ rcc-reboot is binded to "sysreset_syscon" driver => regmap field = handle to rcc

Without modification, the syscon-reboot driver can't have access to regmap of RCC because it is not bound to generic syscon driver.

I have 2 solutions to solve this:

1/ add a 3rd binding to syscon in RCC misc driver if (rcc_clk->soc == STM32MP1) { ret = device_bind_driver_to_node(dev, "syscon", "syscon", dev_ofnode(dev), &child); if (ret) return ret; }

2/ change sysreset to access directly to syscon regmap without explicate binding.

I choose the 2nd solution, I think it is more generic.

NB: I can change the patch to introduce a new syscon function to access to syscon form phandle if it is preferable struct regmap * syscon_handle_to_regmap(struct udevice *parent, const char *name)

or I can come back to the first solution (add the binding to syscon)

Regards, Simon

Regards Patrick