From: Bartosz Golaszewski bartosz.golaszewski@linaro.org
Members of struct software_node_ref_args should not be dereferenced directly but set using the provided macros. Commit d7cdbbc93c56 ("software node: allow referencing firmware nodes") changed the name of the software node member and caused a build failure. Remove all direct dereferences of the ref struct as a fix.
However, this driver also seems to abuse the software node interface by waiting for a node with an arbitrary name "intel-xhci-usb-sw" to appear in the system before setting up the reference for the I2C device, while the actual software node already exists in the intel-xhci-usb-role-switch module and should be used to set up a static reference. Add a FIXME for a future improvement.
Fixes: d7cdbbc93c56 ("software node: allow referencing firmware nodes") Fixes: 53c24c2932e5 ("platform/x86: intel_cht_int33fe: use inline reference properties") Cc: stable@vger.kernel.org Reported-by: Stephen Rothwell sfr@canb.auug.org.au Closes: https://lore.kernel.org/all/20251121111534.7cdbfe5c@canb.auug.org.au/ Signed-off-by: Bartosz Golaszewski bartosz.golaszewski@linaro.org --- This should go into the reset tree as a fix to the regression introduced by the reset-gpio driver rework. --- drivers/platform/x86/intel/chtwc_int33fe.c | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/drivers/platform/x86/intel/chtwc_int33fe.c b/drivers/platform/x86/intel/chtwc_int33fe.c index 29e8b5432f4c9eea7dc45b83d94c0e00373f901b..d183aa53c318ba8d57c7124c38506e6956b3ee36 100644 --- a/drivers/platform/x86/intel/chtwc_int33fe.c +++ b/drivers/platform/x86/intel/chtwc_int33fe.c @@ -77,7 +77,7 @@ static const struct software_node max17047_node = { * software node. */ static struct software_node_ref_args fusb302_mux_refs[] = { - { .node = NULL }, + SOFTWARE_NODE_REFERENCE(NULL), };
static const struct property_entry fusb302_properties[] = { @@ -190,11 +190,6 @@ static void cht_int33fe_remove_nodes(struct cht_int33fe_data *data) { software_node_unregister_node_group(node_group);
- if (fusb302_mux_refs[0].node) { - fwnode_handle_put(software_node_fwnode(fusb302_mux_refs[0].node)); - fusb302_mux_refs[0].node = NULL; - } - if (data->dp) { data->dp->secondary = NULL; fwnode_handle_put(data->dp); @@ -202,7 +197,15 @@ static void cht_int33fe_remove_nodes(struct cht_int33fe_data *data) } }
-static int cht_int33fe_add_nodes(struct cht_int33fe_data *data) +static void cht_int33fe_put_swnode(void *data) +{ + struct fwnode_handle *fwnode = data; + + fwnode_handle_put(fwnode); + fusb302_mux_refs[0] = SOFTWARE_NODE_REFERENCE(NULL); +} + +static int cht_int33fe_add_nodes(struct device *dev, struct cht_int33fe_data *data) { const struct software_node *mux_ref_node; int ret; @@ -212,17 +215,25 @@ static int cht_int33fe_add_nodes(struct cht_int33fe_data *data) * until the mux driver has created software node for the mux device. * It means we depend on the mux driver. This function will return * -EPROBE_DEFER until the mux device is registered. + * + * FIXME: the relevant software node exists in intel-xhci-usb-role-switch + * and - if exported - could be used to set up a static reference. */ mux_ref_node = software_node_find_by_name(NULL, "intel-xhci-usb-sw"); if (!mux_ref_node) return -EPROBE_DEFER;
+ ret = devm_add_action_or_reset(dev, cht_int33fe_put_swnode, + software_node_fwnode(mux_ref_node)); + if (ret) + return ret; + /* * Update node used in "usb-role-switch" property. Note that we * rely on software_node_register_node_group() to use the original * instance of properties instead of copying them. */ - fusb302_mux_refs[0].node = mux_ref_node; + fusb302_mux_refs[0] = SOFTWARE_NODE_REFERENCE(mux_ref_node);
ret = software_node_register_node_group(node_group); if (ret) @@ -345,7 +356,7 @@ static int cht_int33fe_typec_probe(struct platform_device *pdev) return fusb302_irq; }
- ret = cht_int33fe_add_nodes(data); + ret = cht_int33fe_add_nodes(dev, data); if (ret) return ret;
--- base-commit: cba510406ba76569782ead6007a0e4eb5d34a7ab change-id: 20251121-int33fe-swnode-fix-e896da458560
Best regards,
Hi,
On 21-Nov-25 11:04 AM, Bartosz Golaszewski wrote:
From: Bartosz Golaszewski bartosz.golaszewski@linaro.org
Members of struct software_node_ref_args should not be dereferenced directly but set using the provided macros. Commit d7cdbbc93c56 ("software node: allow referencing firmware nodes") changed the name of the software node member and caused a build failure. Remove all direct dereferences of the ref struct as a fix.
However, this driver also seems to abuse the software node interface by waiting for a node with an arbitrary name "intel-xhci-usb-sw" to appear in the system before setting up the reference for the I2C device, while the actual software node already exists in the intel-xhci-usb-role-switch module and should be used to set up a static reference. Add a FIXME for a future improvement.
Fixes: d7cdbbc93c56 ("software node: allow referencing firmware nodes") Fixes: 53c24c2932e5 ("platform/x86: intel_cht_int33fe: use inline reference properties") Cc: stable@vger.kernel.org Reported-by: Stephen Rothwell sfr@canb.auug.org.au Closes: https://lore.kernel.org/all/20251121111534.7cdbfe5c@canb.auug.org.au/ Signed-off-by: Bartosz Golaszewski bartosz.golaszewski@linaro.org
This should go into the reset tree as a fix to the regression introduced by the reset-gpio driver rework.
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede johannes.goede@oss.qualcomm.com
Also ack for merging this through the reset tree.
Ilpo please do *not* pick this one up as it will be merged through the reset tree.
Regards,
Hans
drivers/platform/x86/intel/chtwc_int33fe.c | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/drivers/platform/x86/intel/chtwc_int33fe.c b/drivers/platform/x86/intel/chtwc_int33fe.c index 29e8b5432f4c9eea7dc45b83d94c0e00373f901b..d183aa53c318ba8d57c7124c38506e6956b3ee36 100644 --- a/drivers/platform/x86/intel/chtwc_int33fe.c +++ b/drivers/platform/x86/intel/chtwc_int33fe.c @@ -77,7 +77,7 @@ static const struct software_node max17047_node = {
- software node.
*/ static struct software_node_ref_args fusb302_mux_refs[] = {
- { .node = NULL },
- SOFTWARE_NODE_REFERENCE(NULL),
}; static const struct property_entry fusb302_properties[] = { @@ -190,11 +190,6 @@ static void cht_int33fe_remove_nodes(struct cht_int33fe_data *data) { software_node_unregister_node_group(node_group);
- if (fusb302_mux_refs[0].node) {
fwnode_handle_put(software_node_fwnode(fusb302_mux_refs[0].node));fusb302_mux_refs[0].node = NULL;- }
- if (data->dp) { data->dp->secondary = NULL; fwnode_handle_put(data->dp);
@@ -202,7 +197,15 @@ static void cht_int33fe_remove_nodes(struct cht_int33fe_data *data) } } -static int cht_int33fe_add_nodes(struct cht_int33fe_data *data) +static void cht_int33fe_put_swnode(void *data) +{
- struct fwnode_handle *fwnode = data;
- fwnode_handle_put(fwnode);
- fusb302_mux_refs[0] = SOFTWARE_NODE_REFERENCE(NULL);
+}
+static int cht_int33fe_add_nodes(struct device *dev, struct cht_int33fe_data *data) { const struct software_node *mux_ref_node; int ret; @@ -212,17 +215,25 @@ static int cht_int33fe_add_nodes(struct cht_int33fe_data *data) * until the mux driver has created software node for the mux device. * It means we depend on the mux driver. This function will return * -EPROBE_DEFER until the mux device is registered.
** FIXME: the relevant software node exists in intel-xhci-usb-role-switch */ mux_ref_node = software_node_find_by_name(NULL, "intel-xhci-usb-sw"); if (!mux_ref_node) return -EPROBE_DEFER;* and - if exported - could be used to set up a static reference.
- ret = devm_add_action_or_reset(dev, cht_int33fe_put_swnode,
software_node_fwnode(mux_ref_node));- if (ret)
return ret;- /*
*/
- Update node used in "usb-role-switch" property. Note that we
- rely on software_node_register_node_group() to use the original
- instance of properties instead of copying them.
- fusb302_mux_refs[0].node = mux_ref_node;
- fusb302_mux_refs[0] = SOFTWARE_NODE_REFERENCE(mux_ref_node);
ret = software_node_register_node_group(node_group); if (ret) @@ -345,7 +356,7 @@ static int cht_int33fe_typec_probe(struct platform_device *pdev) return fusb302_irq; }
- ret = cht_int33fe_add_nodes(data);
- ret = cht_int33fe_add_nodes(dev, data); if (ret) return ret;
base-commit: cba510406ba76569782ead6007a0e4eb5d34a7ab change-id: 20251121-int33fe-swnode-fix-e896da458560
Best regards,
On Fri, Nov 21, 2025 at 2:40 PM Hans de Goede hansg@kernel.org wrote:
On 21-Nov-25 11:04 AM, Bartosz Golaszewski wrote:
From: Bartosz Golaszewski bartosz.golaszewski@linaro.org
Members of struct software_node_ref_args should not be dereferenced directly but set using the provided macros. Commit d7cdbbc93c56 ("software node: allow referencing firmware nodes") changed the name of the software node member and caused a build failure. Remove all direct dereferences of the ref struct as a fix.
However, this driver also seems to abuse the software node interface by waiting for a node with an arbitrary name "intel-xhci-usb-sw" to appear in the system before setting up the reference for the I2C device, while the actual software node already exists in the intel-xhci-usb-role-switch module and should be used to set up a static reference. Add a FIXME for a future improvement.
Fixes: d7cdbbc93c56 ("software node: allow referencing firmware nodes") Fixes: 53c24c2932e5 ("platform/x86: intel_cht_int33fe: use inline reference properties") Cc: stable@vger.kernel.org Reported-by: Stephen Rothwell sfr@canb.auug.org.au Closes: https://lore.kernel.org/all/20251121111534.7cdbfe5c@canb.auug.org.au/ Signed-off-by: Bartosz Golaszewski bartosz.golaszewski@linaro.org
This should go into the reset tree as a fix to the regression introduced by the reset-gpio driver rework.
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede johannes.goede@oss.qualcomm.com
Also ack for merging this through the reset tree.
Ilpo please do *not* pick this one up as it will be merged through the reset tree.
Philipp: I'm afraid this too must go into an immutable branch shared with the GPIO tree or else Linus Torvalds will yell at me if by chance he pulls my changes first into mainline. Unless you plan to do your PR early into the merge window in which case I can wait until it's in his tree and submit mine. Let me know what your preference is.
Bart
On Fri, 21 Nov 2025, Hans de Goede wrote:
Hi,
On 21-Nov-25 11:04 AM, Bartosz Golaszewski wrote:
From: Bartosz Golaszewski bartosz.golaszewski@linaro.org
Members of struct software_node_ref_args should not be dereferenced directly but set using the provided macros. Commit d7cdbbc93c56 ("software node: allow referencing firmware nodes") changed the name of the software node member and caused a build failure. Remove all direct dereferences of the ref struct as a fix.
However, this driver also seems to abuse the software node interface by waiting for a node with an arbitrary name "intel-xhci-usb-sw" to appear in the system before setting up the reference for the I2C device, while the actual software node already exists in the intel-xhci-usb-role-switch module and should be used to set up a static reference. Add a FIXME for a future improvement.
Fixes: d7cdbbc93c56 ("software node: allow referencing firmware nodes") Fixes: 53c24c2932e5 ("platform/x86: intel_cht_int33fe: use inline reference properties") Cc: stable@vger.kernel.org Reported-by: Stephen Rothwell sfr@canb.auug.org.au Closes: https://lore.kernel.org/all/20251121111534.7cdbfe5c@canb.auug.org.au/ Signed-off-by: Bartosz Golaszewski bartosz.golaszewski@linaro.org
This should go into the reset tree as a fix to the regression introduced by the reset-gpio driver rework.
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede johannes.goede@oss.qualcomm.com
Also ack for merging this through the reset tree.
Ilpo please do *not* pick this one up as it will be merged through the reset tree.
Fine,
Acked-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com
On Fr, 2025-11-21 at 11:04 +0100, Bartosz Golaszewski wrote:
From: Bartosz Golaszewski bartosz.golaszewski@linaro.org
Members of struct software_node_ref_args should not be dereferenced directly but set using the provided macros. Commit d7cdbbc93c56 ("software node: allow referencing firmware nodes") changed the name of the software node member and caused a build failure. Remove all direct dereferences of the ref struct as a fix.
However, this driver also seems to abuse the software node interface by waiting for a node with an arbitrary name "intel-xhci-usb-sw" to appear in the system before setting up the reference for the I2C device, while the actual software node already exists in the intel-xhci-usb-role-switch module and should be used to set up a static reference. Add a FIXME for a future improvement.
Fixes: d7cdbbc93c56 ("software node: allow referencing firmware nodes") Fixes: 53c24c2932e5 ("platform/x86: intel_cht_int33fe: use inline reference properties") Cc: stable@vger.kernel.org Reported-by: Stephen Rothwell sfr@canb.auug.org.au Closes: https://lore.kernel.org/all/20251121111534.7cdbfe5c@canb.auug.org.au/ Signed-off-by: Bartosz Golaszewski bartosz.golaszewski@linaro.org
This should go into the reset tree as a fix to the regression introduced by the reset-gpio driver rework.
Applied to reset/gpio, thanks!
[1/1] platform/x86: intel: chtwc_int33fe: don't dereference swnode args https://git.pengutronix.de/cgit/pza/linux/commit/?id=527250cd9092
regards Philipp
Hi Bartosz,
On Fr, 2025-11-21 at 14:50 +0100, Bartosz Golaszewski wrote:
On Fri, Nov 21, 2025 at 2:40 PM Hans de Goede hansg@kernel.org wrote:
On 21-Nov-25 11:04 AM, Bartosz Golaszewski wrote:
From: Bartosz Golaszewski bartosz.golaszewski@linaro.org
Members of struct software_node_ref_args should not be dereferenced directly but set using the provided macros. Commit d7cdbbc93c56 ("software node: allow referencing firmware nodes") changed the name of the software node member and caused a build failure. Remove all direct dereferences of the ref struct as a fix.
However, this driver also seems to abuse the software node interface by waiting for a node with an arbitrary name "intel-xhci-usb-sw" to appear in the system before setting up the reference for the I2C device, while the actual software node already exists in the intel-xhci-usb-role-switch module and should be used to set up a static reference. Add a FIXME for a future improvement.
Fixes: d7cdbbc93c56 ("software node: allow referencing firmware nodes") Fixes: 53c24c2932e5 ("platform/x86: intel_cht_int33fe: use inline reference properties") Cc: stable@vger.kernel.org Reported-by: Stephen Rothwell sfr@canb.auug.org.au Closes: https://lore.kernel.org/all/20251121111534.7cdbfe5c@canb.auug.org.au/ Signed-off-by: Bartosz Golaszewski bartosz.golaszewski@linaro.org
This should go into the reset tree as a fix to the regression introduced by the reset-gpio driver rework.
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede johannes.goede@oss.qualcomm.com
Also ack for merging this through the reset tree.
Ilpo please do *not* pick this one up as it will be merged through the reset tree.
Philipp: I'm afraid this too must go into an immutable branch shared with the GPIO tree or else Linus Torvalds will yell at me if by chance he pulls my changes first into mainline. Unless you plan to do your PR early into the merge window in which case I can wait until it's in his tree and submit mine. Let me know what your preference is.
The following changes since commit 5fc4e4cf7a2268b5f73700fd1e8d02159f2417d8:
reset: gpio: use software nodes to setup the GPIO lookup (2025-11-20 16:51:49 +0100)
are available in the Git repository at:
https://git.pengutronix.de/git/pza/linux.git tags/reset-gpio-for-v6.19-2
for you to fetch changes up to 527250cd9092461f1beac3e4180a4481bffa01b5:
platform/x86: intel: chtwc_int33fe: don't dereference swnode args (2025-11-21 15:31:43 +0100)
---------------------------------------------------------------- Reset/GPIO/swnode changes for v6.19 (v2)
* Fix chtwc_int33fe build issue since commit d7cdbbc93c56 ("software node: allow referencing firmware nodes").
---------------------------------------------------------------- Bartosz Golaszewski (1): platform/x86: intel: chtwc_int33fe: don't dereference swnode args
drivers/platform/x86/intel/chtwc_int33fe.c | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-)
linux-stable-mirror@lists.linaro.org