| Message ID | 20170803144813.28010-1-c_mykolak@mellanox.com |
|---|---|
| State | Rejected, archived |
| Headers | show |
Hey Mykola, On Fri, Aug 4, 2017 at 12:48 AM, Mykola Kostenok <c_mykolak@mellanox.com> wrote: > Add CONFIG_OF_DYNAMIC for hotplug operations. > It requires CONFIG_OF_UNITTEST so it added too. > > Signed-off-by: Mykola Kostenok <c_mykolak@mellanox.com> > --- > arch/arm/configs/aspeed_g5_defconfig | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/arm/configs/aspeed_g5_defconfig b/arch/arm/configs/aspeed_g5_defconfig > index 2025e45..e78e306 100644 > --- a/arch/arm/configs/aspeed_g5_defconfig > +++ b/arch/arm/configs/aspeed_g5_defconfig > @@ -215,3 +215,5 @@ CONFIG_DEBUG_USER=y > # CONFIG_XZ_DEC_POWERPC is not set > # CONFIG_XZ_DEC_IA64 is not set > # CONFIG_XZ_DEC_SPARC is not set > +CONFIG_OF_UNITTEST=y > +CONFIG_OF_DYNAMIC=y I'm not sure that this is something we want enabled: CONFIG_OF_UNITTEST: This option builds in test cases for the device tree infrastructure that are executed once at boot time, and the results dumped to the console. CONFIG_OF_DYNAMIC: On some platforms, the device tree can be manipulated at runtime. While this option is selected automatically on such platforms, you can enable it manually to improve device tree unit test coverage. Can you explain what you're wanting to do here? I've added some of our device tree experts to cc so they can offer advice. Cheers, Joel
> -----Original Message----- > From: joel.stan@gmail.com [mailto:joel.stan@gmail.com] On Behalf Of Joel > Stanley > Sent: Tuesday, August 8, 2017 4:09 PM > To: Mykola Kostenok <c_mykolak@mellanox.com> > Cc: OpenBMC Maillist <openbmc@lists.ozlabs.org>; Vadim Pasternak > <vadimp@mellanox.com>; Ohad Oz <ohado@mellanox.com>; Benjamin > Herrenschmidt <benh@kernel.crashing.org>; Michael Ellerman > <mpe@ellerman.id.au>; Jeremy Kerr <jk@ozlabs.org> > Subject: Re: [PATCH linux dev-4.10] aspeed_g5_defconfig: add > CONFIG_OF_DYNAMIC. > > Hey Mykola, > > On Fri, Aug 4, 2017 at 12:48 AM, Mykola Kostenok > <c_mykolak@mellanox.com> wrote: > > Add CONFIG_OF_DYNAMIC for hotplug operations. > > It requires CONFIG_OF_UNITTEST so it added too. > > > > Signed-off-by: Mykola Kostenok <c_mykolak@mellanox.com> > > --- > > arch/arm/configs/aspeed_g5_defconfig | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/arch/arm/configs/aspeed_g5_defconfig > > b/arch/arm/configs/aspeed_g5_defconfig > > index 2025e45..e78e306 100644 > > --- a/arch/arm/configs/aspeed_g5_defconfig > > +++ b/arch/arm/configs/aspeed_g5_defconfig > > @@ -215,3 +215,5 @@ CONFIG_DEBUG_USER=y # > CONFIG_XZ_DEC_POWERPC is > > not set # CONFIG_XZ_DEC_IA64 is not set # CONFIG_XZ_DEC_SPARC is > not > > set > > +CONFIG_OF_UNITTEST=y > > +CONFIG_OF_DYNAMIC=y > > I'm not sure that this is something we want enabled: > > CONFIG_OF_UNITTEST: > This option builds in test cases for the device tree infrastructure that are > executed once at boot time, and the results dumped to the console. > > CONFIG_OF_DYNAMIC: > On some platforms, the device tree can be manipulated at runtime. > While this option is selected automatically on such platforms, you can > enable it manually to improve device tree unit test coverage. > > Can you explain what you're wanting to do here? > > I've added some of our device tree experts to cc so they can offer advice. > > Cheers, > > Joel Hi, Joel. We have now patchset supporting our CPLD device in upstream review. There we have support for dynamic (hotplug) devices, like PSUs, FANs, ASIC, etc. These devices are described in DTS like, f.e. &i2c3 { status = "okay"; #address-cells = <1>; #size-cells = <0>; psu1_eeprom: eeprom@51 { compatible = "atmel,24c02"; reg = <0x51>; status = "disabled"; }; psu2_eeprom: eeprom@50 { compatible = "atmel,24c02"; reg = <0x50>; status = "disabled"; }; psu1_ctrl: dps460@59 { compatible = "dps460"; reg = <0x59>; status = "disabled"; }; psu2_ctrl: dps460@58 { compatible = "dps460"; reg = <0x58>; status = "disabled"; }; }; &i2c12 { status = "okay"; #address-cells = <1>; #size-cells = <0>; asic_thermal: mlxsw_minimal@48 { compatible = "mlxsw_minimal"; reg = <0x48>; status = "disabled"; cooling-phandle = <&cooling>; trips { /* trip type, temp in mcelsius, temp min, temp max */ trip@0 { trip = <0 75000 0 0>; }; trip@1 { trip = <2 85000 1 5>; }; trip@3 { trip = <2 105000 5 5>; }; }; }; }; They all sets with the state "disabled", and driver activates it with the below calls, when related hotplug event is received in interrupt handler: static struct property mlxreg_core_device_dis = { .name = MLXREG_CORE_PROP_STATUS, .value = MLXREG_CORE_PROP_DISABLED, .length = sizeof(MLXREG_CORE_PROP_DISABLED), }; static void mlxreg_core_dev_enable(struct device_node *np) { /* Enable and create device. */ of_update_property(np, &mlxreg_core_device_en); } static void mlxreg_core_dev_disable(struct device_node *np) { /* Disable and unregister platform device. */ of_update_property(np, &mlxreg_core_device_dis); of_node_clear_flag(np, OF_POPULATED); } For that we must have OF_DYNAMIC set, and last one required OF_UNITEST. Best regards. Mykola Kostenok.
Mykola Kostenok <c_mykolak@mellanox.com> writes: >> -----Original Message----- >> From: joel.stan@gmail.com [mailto:joel.stan@gmail.com] On Behalf Of Joel >> Stanley >> >> Can you explain what you're wanting to do here? >> >> I've added some of our device tree experts to cc so they can offer advice. > > Hi, Joel. > > We have now patchset supporting our CPLD device in upstream review. > There we have support for dynamic (hotplug) devices, like PSUs, FANs, ASIC, etc. > ... > > They all sets with the state "disabled", and driver activates it with the below calls, when related hotplug event is received in interrupt handler: > > static struct property mlxreg_core_device_dis = { > .name = MLXREG_CORE_PROP_STATUS, > .value = MLXREG_CORE_PROP_DISABLED, > .length = sizeof(MLXREG_CORE_PROP_DISABLED), > }; > > static void mlxreg_core_dev_enable(struct device_node *np) { > /* Enable and create device. */ > of_update_property(np, &mlxreg_core_device_en); } > > static void mlxreg_core_dev_disable(struct device_node *np) { > /* Disable and unregister platform device. */ > of_update_property(np, &mlxreg_core_device_dis); > of_node_clear_flag(np, OF_POPULATED); > } > > For that we must have OF_DYNAMIC set, and last one required OF_UNITEST. But are you actually changing the structure of the device tree at run time? If all you're doing is using of_update_property() to change the status property I don't think you need OF_DYNAMIC. cheers
> -----Original Message----- > From: Michael Ellerman [mailto:mpe@ellerman.id.au] > Sent: Wednesday, August 9, 2017 4:01 PM > To: Mykola Kostenok <c_mykolak@mellanox.com>; Joel Stanley > <joel@jms.id.au> > Cc: OpenBMC Maillist <openbmc@lists.ozlabs.org>; Vadim Pasternak > <vadimp@mellanox.com>; Ohad Oz <ohado@mellanox.com>; Benjamin > Herrenschmidt <benh@kernel.crashing.org>; Jeremy Kerr <jk@ozlabs.org> > Subject: RE: [PATCH linux dev-4.10] aspeed_g5_defconfig: add > CONFIG_OF_DYNAMIC. > > Mykola Kostenok <c_mykolak@mellanox.com> writes: > >> -----Original Message----- > >> From: joel.stan@gmail.com [mailto:joel.stan@gmail.com] On Behalf Of > >> Joel >> Stanley > >> > >> Can you explain what you're wanting to do here? > >> > >> I've added some of our device tree experts to cc so they can offer advice. > > > > Hi, Joel. > > > > We have now patchset supporting our CPLD device in upstream review. > > There we have support for dynamic (hotplug) devices, like PSUs, FANs, > ASIC, etc. > > > ... > > > > They all sets with the state "disabled", and driver activates it with the > below calls, when related hotplug event is received in interrupt handler: > > > > static struct property mlxreg_core_device_dis = { > > .name = MLXREG_CORE_PROP_STATUS, > > .value = MLXREG_CORE_PROP_DISABLED, > > .length = sizeof(MLXREG_CORE_PROP_DISABLED), > > }; > > > > static void mlxreg_core_dev_enable(struct device_node *np) { > > /* Enable and create device. */ > > of_update_property(np, &mlxreg_core_device_en); } > > > > static void mlxreg_core_dev_disable(struct device_node *np) { > > /* Disable and unregister platform device. */ > > of_update_property(np, &mlxreg_core_device_dis); > > of_node_clear_flag(np, OF_POPULATED); } > > > > For that we must have OF_DYNAMIC set, and last one required > OF_UNITEST. > > But are you actually changing the structure of the device tree at run time? > > If all you're doing is using of_update_property() to change the status > property I don't think you need OF_DYNAMIC. > > cheers Hi, Michael. In drivers/i2c/i2c-core.c: #if IS_ENABLED(CONFIG_OF_DYNAMIC) static int of_i2c_notify(struct notifier_block *nb, unsigned long action, void *arg) So we need it. Best regards. Mykola Kostenok.
diff --git a/arch/arm/configs/aspeed_g5_defconfig b/arch/arm/configs/aspeed_g5_defconfig index 2025e45..e78e306 100644 --- a/arch/arm/configs/aspeed_g5_defconfig +++ b/arch/arm/configs/aspeed_g5_defconfig @@ -215,3 +215,5 @@ CONFIG_DEBUG_USER=y # CONFIG_XZ_DEC_POWERPC is not set # CONFIG_XZ_DEC_IA64 is not set # CONFIG_XZ_DEC_SPARC is not set +CONFIG_OF_UNITTEST=y +CONFIG_OF_DYNAMIC=y
Add CONFIG_OF_DYNAMIC for hotplug operations. It requires CONFIG_OF_UNITTEST so it added too. Signed-off-by: Mykola Kostenok <c_mykolak@mellanox.com> --- arch/arm/configs/aspeed_g5_defconfig | 2 ++ 1 file changed, 2 insertions(+)