| Message ID | 20190906043809.18990-2-dmitry.torokhov@gmail.com (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
| Series | [1/3] software node: implement reference properties | expand |
Hi, On Thu, Sep 05, 2019 at 09:38:08PM -0700, Dmitry Torokhov wrote: > Now that static device properties allow defining reference properties > together with all other types of properties, instead of managing them > separately, let's adjust the driver. > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > --- > > Heikki, I do not have this hardware, so if you could try this out > it would be really great. > > drivers/platform/x86/intel_cht_int33fe.c | 46 ++++++++++++------------ > 1 file changed, 22 insertions(+), 24 deletions(-) > > diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c > index 4fbdff48a4b5..91f3c8840fd8 100644 > --- a/drivers/platform/x86/intel_cht_int33fe.c > +++ b/drivers/platform/x86/intel_cht_int33fe.c > @@ -50,28 +50,8 @@ struct cht_int33fe_data { > > static const struct software_node nodes[]; I think you can remove that. > -static const struct software_node_ref_args pi3usb30532_ref = { > - &nodes[INT33FE_NODE_PI3USB30532] > -}; > - > -static const struct software_node_ref_args dp_ref = { > - &nodes[INT33FE_NODE_DISPLAYPORT] > -}; > - > static struct software_node_ref_args mux_ref; I'm pretty sure you should now drop that one. > -static const struct software_node_reference usb_connector_refs[] = { > - { "orientation-switch", 1, &pi3usb30532_ref}, > - { "mode-switch", 1, &pi3usb30532_ref}, > - { "displayport", 1, &dp_ref}, > - { } > -}; > - > -static const struct software_node_reference fusb302_refs[] = { > - { "usb-role-switch", 1, &mux_ref}, > - { } > -}; > - > /* > * Grrr I severly dislike buggy BIOS-es. At least one BIOS enumerates > * the max17047 both through the INT33FE ACPI device (it is right there > @@ -107,7 +87,13 @@ static const struct property_entry max17047_props[] = { > { } > }; > > -static const struct property_entry fusb302_props[] = { > +/* Not const because we need to update "usb-role-switch" property. */ > +static struct property_entry fusb302_props[] = { > + /* > + * usb-role-switch property must be first as we rely on fixed > + * position to adjust it once we know the real node. > + */ > + PROPERTY_ENTRY_REF("usb-role-switch", NULL), > PROPERTY_ENTRY_STRING("linux,extcon-name", "cht_wcove_pwrsrc"), > { } > }; > @@ -131,16 +117,22 @@ static const struct property_entry usb_connector_props[] = { > PROPERTY_ENTRY_U32_ARRAY("source-pdos", src_pdo), > PROPERTY_ENTRY_U32_ARRAY("sink-pdos", snk_pdo), > PROPERTY_ENTRY_U32("op-sink-microwatt", 2500000), > + PROPERTY_ENTRY_REF("orientation-switch", > + &nodes[INT33FE_NODE_PI3USB30532]), > + PROPERTY_ENTRY_REF("mode-switch", > + &nodes[INT33FE_NODE_PI3USB30532]), > + PROPERTY_ENTRY_REF("displayport", > + &nodes[INT33FE_NODE_DISPLAYPORT]), > { } > }; > > static const struct software_node nodes[] = { > - { "fusb302", NULL, fusb302_props, fusb302_refs }, > + { "fusb302", NULL, fusb302_props }, > { "max17047", NULL, max17047_props }, > { "pi3usb30532" }, > { "displayport" }, > { "usb-role-switch" }, > - { "connector", &nodes[0], usb_connector_props, usb_connector_refs }, > + { "connector", &nodes[0], usb_connector_props }, > { } > }; > > @@ -174,7 +166,13 @@ static int cht_int33fe_setup_mux(struct cht_int33fe_data *data) > > data->mux = fwnode_handle_get(dev->fwnode); > put_device(dev); > - mux_ref.node = to_software_node(data->mux); > + > + /* > + * Update "usb-role-switch" property with real node. Note that we > + * rely on software_node_register_nodes() to use the original > + * instance of properties instead of copying them. > + */ > + fusb302_props[0].value.ref.node = to_software_node(data->mux); There are other changes to this driver and to swnode.c in Rafael's tree, so if you send v2 soon, then please rebase on top of his devprop branch, or alternatively linux-next. thanks,
On Fri, Sep 06, 2019 at 02:22:43PM +0300, Heikki Krogerus wrote: > Hi, > > On Thu, Sep 05, 2019 at 09:38:08PM -0700, Dmitry Torokhov wrote: > > Now that static device properties allow defining reference properties > > together with all other types of properties, instead of managing them > > separately, let's adjust the driver. > > > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > --- > > > > Heikki, I do not have this hardware, so if you could try this out > > it would be really great. > > > > drivers/platform/x86/intel_cht_int33fe.c | 46 ++++++++++++------------ > > 1 file changed, 22 insertions(+), 24 deletions(-) > > > > diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c > > index 4fbdff48a4b5..91f3c8840fd8 100644 > > --- a/drivers/platform/x86/intel_cht_int33fe.c > > +++ b/drivers/platform/x86/intel_cht_int33fe.c > > @@ -50,28 +50,8 @@ struct cht_int33fe_data { > > > > static const struct software_node nodes[]; > > I think you can remove that. Not really, as there is still circular dependency between nodes and properties. I moved it down closer to properties though. > > > -static const struct software_node_ref_args pi3usb30532_ref = { > > - &nodes[INT33FE_NODE_PI3USB30532] > > -}; > > - > > -static const struct software_node_ref_args dp_ref = { > > - &nodes[INT33FE_NODE_DISPLAYPORT] > > -}; > > - > > static struct software_node_ref_args mux_ref; > > I'm pretty sure you should now drop that one. I ended up reworking things a bit and still using a form of this to reach in and change data, as properties ended up being constants. > > > -static const struct software_node_reference usb_connector_refs[] = { > > - { "orientation-switch", 1, &pi3usb30532_ref}, > > - { "mode-switch", 1, &pi3usb30532_ref}, > > - { "displayport", 1, &dp_ref}, > > - { } > > -}; > > - > > -static const struct software_node_reference fusb302_refs[] = { > > - { "usb-role-switch", 1, &mux_ref}, > > - { } > > -}; > > - > > /* > > * Grrr I severly dislike buggy BIOS-es. At least one BIOS enumerates > > * the max17047 both through the INT33FE ACPI device (it is right there > > @@ -107,7 +87,13 @@ static const struct property_entry max17047_props[] = { > > { } > > }; > > > > -static const struct property_entry fusb302_props[] = { > > +/* Not const because we need to update "usb-role-switch" property. */ > > +static struct property_entry fusb302_props[] = { > > + /* > > + * usb-role-switch property must be first as we rely on fixed > > + * position to adjust it once we know the real node. > > + */ > > + PROPERTY_ENTRY_REF("usb-role-switch", NULL), > > PROPERTY_ENTRY_STRING("linux,extcon-name", "cht_wcove_pwrsrc"), > > { } > > }; > > @@ -131,16 +117,22 @@ static const struct property_entry usb_connector_props[] = { > > PROPERTY_ENTRY_U32_ARRAY("source-pdos", src_pdo), > > PROPERTY_ENTRY_U32_ARRAY("sink-pdos", snk_pdo), > > PROPERTY_ENTRY_U32("op-sink-microwatt", 2500000), > > + PROPERTY_ENTRY_REF("orientation-switch", > > + &nodes[INT33FE_NODE_PI3USB30532]), > > + PROPERTY_ENTRY_REF("mode-switch", > > + &nodes[INT33FE_NODE_PI3USB30532]), > > + PROPERTY_ENTRY_REF("displayport", > > + &nodes[INT33FE_NODE_DISPLAYPORT]), > > { } > > }; > > > > static const struct software_node nodes[] = { > > - { "fusb302", NULL, fusb302_props, fusb302_refs }, > > + { "fusb302", NULL, fusb302_props }, > > { "max17047", NULL, max17047_props }, > > { "pi3usb30532" }, > > { "displayport" }, > > { "usb-role-switch" }, > > - { "connector", &nodes[0], usb_connector_props, usb_connector_refs }, > > + { "connector", &nodes[0], usb_connector_props }, > > { } > > }; > > > > @@ -174,7 +166,13 @@ static int cht_int33fe_setup_mux(struct cht_int33fe_data *data) > > > > data->mux = fwnode_handle_get(dev->fwnode); > > put_device(dev); > > - mux_ref.node = to_software_node(data->mux); > > + > > + /* > > + * Update "usb-role-switch" property with real node. Note that we > > + * rely on software_node_register_nodes() to use the original > > + * instance of properties instead of copying them. > > + */ > > + fusb302_props[0].value.ref.node = to_software_node(data->mux); > > There are other changes to this driver and to swnode.c in Rafael's > tree, so if you send v2 soon, then please rebase on top of his devprop > branch, or alternatively linux-next. Yep, did that, will re-post series shortly.
diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c index 4fbdff48a4b5..91f3c8840fd8 100644 --- a/drivers/platform/x86/intel_cht_int33fe.c +++ b/drivers/platform/x86/intel_cht_int33fe.c @@ -50,28 +50,8 @@ struct cht_int33fe_data { static const struct software_node nodes[]; -static const struct software_node_ref_args pi3usb30532_ref = { - &nodes[INT33FE_NODE_PI3USB30532] -}; - -static const struct software_node_ref_args dp_ref = { - &nodes[INT33FE_NODE_DISPLAYPORT] -}; - static struct software_node_ref_args mux_ref; -static const struct software_node_reference usb_connector_refs[] = { - { "orientation-switch", 1, &pi3usb30532_ref}, - { "mode-switch", 1, &pi3usb30532_ref}, - { "displayport", 1, &dp_ref}, - { } -}; - -static const struct software_node_reference fusb302_refs[] = { - { "usb-role-switch", 1, &mux_ref}, - { } -}; - /* * Grrr I severly dislike buggy BIOS-es. At least one BIOS enumerates * the max17047 both through the INT33FE ACPI device (it is right there @@ -107,7 +87,13 @@ static const struct property_entry max17047_props[] = { { } }; -static const struct property_entry fusb302_props[] = { +/* Not const because we need to update "usb-role-switch" property. */ +static struct property_entry fusb302_props[] = { + /* + * usb-role-switch property must be first as we rely on fixed + * position to adjust it once we know the real node. + */ + PROPERTY_ENTRY_REF("usb-role-switch", NULL), PROPERTY_ENTRY_STRING("linux,extcon-name", "cht_wcove_pwrsrc"), { } }; @@ -131,16 +117,22 @@ static const struct property_entry usb_connector_props[] = { PROPERTY_ENTRY_U32_ARRAY("source-pdos", src_pdo), PROPERTY_ENTRY_U32_ARRAY("sink-pdos", snk_pdo), PROPERTY_ENTRY_U32("op-sink-microwatt", 2500000), + PROPERTY_ENTRY_REF("orientation-switch", + &nodes[INT33FE_NODE_PI3USB30532]), + PROPERTY_ENTRY_REF("mode-switch", + &nodes[INT33FE_NODE_PI3USB30532]), + PROPERTY_ENTRY_REF("displayport", + &nodes[INT33FE_NODE_DISPLAYPORT]), { } }; static const struct software_node nodes[] = { - { "fusb302", NULL, fusb302_props, fusb302_refs }, + { "fusb302", NULL, fusb302_props }, { "max17047", NULL, max17047_props }, { "pi3usb30532" }, { "displayport" }, { "usb-role-switch" }, - { "connector", &nodes[0], usb_connector_props, usb_connector_refs }, + { "connector", &nodes[0], usb_connector_props }, { } }; @@ -174,7 +166,13 @@ static int cht_int33fe_setup_mux(struct cht_int33fe_data *data) data->mux = fwnode_handle_get(dev->fwnode); put_device(dev); - mux_ref.node = to_software_node(data->mux); + + /* + * Update "usb-role-switch" property with real node. Note that we + * rely on software_node_register_nodes() to use the original + * instance of properties instead of copying them. + */ + fusb302_props[0].value.ref.node = to_software_node(data->mux); return 0; }
Now that static device properties allow defining reference properties together with all other types of properties, instead of managing them separately, let's adjust the driver. Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> --- Heikki, I do not have this hardware, so if you could try this out it would be really great. drivers/platform/x86/intel_cht_int33fe.c | 46 ++++++++++++------------ 1 file changed, 22 insertions(+), 24 deletions(-)