* [PATCH v18 1/5] phy: core: Fix race-condition between _of_phy_get() and try_module_get()
2026-09-17 11:07 [PATCH v18 0/5] Add phy_get_by_of_node and devm helper Bryan O'Donoghue
@ 2026-09-17 11:07 ` Bryan O'Donoghue
2026-09-17 11:07 ` [PATCH v18 2/5] phy: core: Add phy_get_by_of_node() Bryan O'Donoghue
` (3 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Bryan O'Donoghue @ 2026-09-17 11:07 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Foss, Todor Tomov,
Mauro Carvalho Chehab, Konrad Dybcio, Vladimir Zapolskiy,
Bryan O'Donoghue, Loic Poulain, Vinod Koul, Neil Armstrong,
Greg Kroah-Hartman, Kishon Vijay Abraham I, Felipe Balbi,
Manivannan Sadhasivam
Cc: linux-arm-msm, linux-clk, devicetree, linux-kernel, linux-media,
linux-phy, Bryan O'Donoghue, Krzysztof Kozlowski, stable,
Manivannan Sadhasivam
Sashiko asked during a patch review if the existing usage pattern had a
race condition; specifically in of_phy_get() if it was possible between
returning from _of_phy_get() and running try_module_get() that a module
might be unbound leading to use-after-free.
Looking at the code this appears to be so, there is no linkage between the
phy and module under a synchronisation primitive.
Using the phy_provider_mutex in phy_get() will ensure there is a link between
the returned phy pointer and the module_get() bumping the module reference
count.
Amend phy_get(), of_phy_get() and devm_of_phy_get_by_index() to fix the
same usage pattern.
phy_provider_unregister() must take the phy_provider_mutex so amending
phy_get()/of_phy_get() to take that same mutex guarantees there is no
use-after-free.
Fixes: ff764963479a1 ("drivers: phy: add generic PHY framework")
Cc: stable@vger.kernel.org
Reviewed-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
drivers/phy/phy-core.c | 45 ++++++++++++++++++++++++++++++---------------
1 file changed, 30 insertions(+), 15 deletions(-)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 21aaf2f76e53e..89addd732bff3 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -124,13 +124,13 @@ static struct phy *phy_find(struct device *dev, const char *con_id)
const char *dev_id = dev_name(dev);
struct phy_lookup *p, *pl = NULL;
- mutex_lock(&phy_provider_mutex);
+ lockdep_assert_held(&phy_provider_mutex);
+
list_for_each_entry(p, &phys, node)
if (!strcmp(p->dev_id, dev_id) && !strcmp(p->con_id, con_id)) {
pl = p;
break;
}
- mutex_unlock(&phy_provider_mutex);
return pl ? pl->phy : ERR_PTR(-ENODEV);
}
@@ -624,6 +624,8 @@ static struct phy *_of_phy_get(struct device_node *np, int index)
struct phy *phy = NULL;
struct of_phandle_args args;
+ lockdep_assert_held(&phy_provider_mutex);
+
ret = of_parse_phandle_with_args(np, "phys", "#phy-cells",
index, &args);
if (ret)
@@ -635,11 +637,10 @@ static struct phy *_of_phy_get(struct device_node *np, int index)
goto out_put_node;
}
- mutex_lock(&phy_provider_mutex);
phy_provider = of_phy_provider_lookup(args.np);
if (IS_ERR(phy_provider) || !try_module_get(phy_provider->owner)) {
phy = ERR_PTR(-EPROBE_DEFER);
- goto out_unlock;
+ goto out_put_node;
}
if (!of_device_is_available(args.np)) {
@@ -653,8 +654,6 @@ static struct phy *_of_phy_get(struct device_node *np, int index)
out_put_module:
module_put(phy_provider->owner);
-out_unlock:
- mutex_unlock(&phy_provider_mutex);
out_put_node:
of_node_put(args.np);
@@ -678,15 +677,21 @@ struct phy *of_phy_get(struct device_node *np, const char *con_id)
if (con_id)
index = of_property_match_string(np, "phy-names", con_id);
+ mutex_lock(&phy_provider_mutex);
+
phy = _of_phy_get(np, index);
if (IS_ERR(phy))
- return phy;
+ goto out_unlock;
- if (!try_module_get(phy->ops->owner))
- return ERR_PTR(-EPROBE_DEFER);
+ if (!try_module_get(phy->ops->owner)) {
+ phy = ERR_PTR(-EPROBE_DEFER);
+ goto out_unlock;
+ }
get_device(&phy->dev);
+out_unlock:
+ mutex_unlock(&phy_provider_mutex);
return phy;
}
EXPORT_SYMBOL_GPL(of_phy_get);
@@ -786,6 +791,7 @@ struct phy *phy_get(struct device *dev, const char *string)
struct phy *phy;
struct device_link *link;
+ mutex_lock(&phy_provider_mutex);
if (dev->of_node) {
if (string)
index = of_property_match_string(dev->of_node, "phy-names",
@@ -796,15 +802,18 @@ struct phy *phy_get(struct device *dev, const char *string)
} else {
if (string == NULL) {
dev_WARN(dev, "missing string\n");
- return ERR_PTR(-EINVAL);
+ phy = ERR_PTR(-EINVAL);
+ goto out_unlock;
}
phy = phy_find(dev, string);
}
if (IS_ERR(phy))
- return phy;
+ goto out_unlock;
- if (!try_module_get(phy->ops->owner))
- return ERR_PTR(-EPROBE_DEFER);
+ if (!try_module_get(phy->ops->owner)) {
+ phy = ERR_PTR(-EPROBE_DEFER);
+ goto out_unlock;
+ }
get_device(&phy->dev);
@@ -813,6 +822,8 @@ struct phy *phy_get(struct device *dev, const char *string)
dev_dbg(dev, "failed to create device link to %s\n",
dev_name(phy->dev.parent));
+out_unlock:
+ mutex_unlock(&phy_provider_mutex);
return phy;
}
EXPORT_SYMBOL_GPL(phy_get);
@@ -961,15 +972,17 @@ struct phy *devm_of_phy_get_by_index(struct device *dev, struct device_node *np,
if (!ptr)
return ERR_PTR(-ENOMEM);
+ mutex_lock(&phy_provider_mutex);
phy = _of_phy_get(np, index);
if (IS_ERR(phy)) {
devres_free(ptr);
- return phy;
+ goto out_unlock;
}
if (!try_module_get(phy->ops->owner)) {
devres_free(ptr);
- return ERR_PTR(-EPROBE_DEFER);
+ phy = ERR_PTR(-EPROBE_DEFER);
+ goto out_unlock;
}
get_device(&phy->dev);
@@ -982,6 +995,8 @@ struct phy *devm_of_phy_get_by_index(struct device *dev, struct device_node *np,
dev_dbg(dev, "failed to create device link to %s\n",
dev_name(phy->dev.parent));
+out_unlock:
+ mutex_unlock(&phy_provider_mutex);
return phy;
}
EXPORT_SYMBOL_GPL(devm_of_phy_get_by_index);
--
2.55.0
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v18 2/5] phy: core: Add phy_get_by_of_node()
2026-09-17 11:07 [PATCH v18 0/5] Add phy_get_by_of_node and devm helper Bryan O'Donoghue
2026-09-17 11:07 ` [PATCH v18 1/5] phy: core: Fix race-condition between _of_phy_get() and try_module_get() Bryan O'Donoghue
@ 2026-09-17 11:07 ` Bryan O'Donoghue
2026-09-17 12:29 ` Neil Armstrong
2026-09-17 11:07 ` [PATCH v18 3/5] phy: core: Add devm_phy_get_by_of_node() Bryan O'Donoghue
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Bryan O'Donoghue @ 2026-09-17 11:07 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Foss, Todor Tomov,
Mauro Carvalho Chehab, Konrad Dybcio, Vladimir Zapolskiy,
Bryan O'Donoghue, Loic Poulain, Vinod Koul, Neil Armstrong,
Greg Kroah-Hartman, Kishon Vijay Abraham I, Felipe Balbi,
Manivannan Sadhasivam
Cc: linux-arm-msm, linux-clk, devicetree, linux-kernel, linux-media,
linux-phy, Bryan O'Donoghue, Krzysztof Kozlowski
Add new function phy_get_by_of_node() allowing lookup of a phy by
device_node. Separates existing logic in _of_phy_get() into an internal
helper method _of_phy_get_with_args() to allow for reuse in new method.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
drivers/phy/phy-core.c | 95 +++++++++++++++++++++++++++++++++++++------------
include/linux/phy/phy.h | 6 ++++
2 files changed, 78 insertions(+), 23 deletions(-)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 89addd732bff3..490a7cde6d40a 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -606,22 +606,51 @@ int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
}
EXPORT_SYMBOL_GPL(phy_validate);
+/**
+ * _of_phy_get_with_args() - lookup and obtain a reference to a phy by of_phandle_args
+ * @args: of_phandle_args to the phy
+ *
+ * Returns the phy from the provider's of_xlate, -ENODEV if disabled,
+ * -EPROBE_DEFER if the provider is not yet registered.
+ */
+static struct phy *_of_phy_get_with_args(struct of_phandle_args *args)
+{
+ struct phy *phy;
+ struct phy_provider *phy_provider;
+
+ lockdep_assert_held(&phy_provider_mutex);
+
+ phy_provider = of_phy_provider_lookup(args->np);
+ if (IS_ERR(phy_provider) || !try_module_get(phy_provider->owner))
+ return ERR_PTR(-EPROBE_DEFER);
+
+ if (!of_device_is_available(args->np)) {
+ dev_warn(phy_provider->dev, "Requested PHY is disabled\n");
+ phy = ERR_PTR(-ENODEV);
+ goto out_put_module;
+ }
+
+ phy = phy_provider->of_xlate(phy_provider->dev, args);
+
+out_put_module:
+ module_put(phy_provider->owner);
+
+ return phy;
+}
+
/**
* _of_phy_get() - lookup and obtain a reference to a phy by phandle
* @np: device_node for which to get the phy
* @index: the index of the phy
*
- * Returns the phy associated with the given phandle value,
- * after getting a refcount to it or -ENODEV if there is no such phy or
- * -EPROBE_DEFER if there is a phandle to the phy, but the device is
- * not yet loaded. This function uses of_xlate call back function provided
- * while registering the phy_provider to find the phy instance.
+ * Returns the phy associated with the given phandle value after getting
+ * a refcount to it; -ENODEV if there is no such phy or the phy is
+ * disabled; -EPROBE_DEFER if the phy provider is not yet available.
*/
static struct phy *_of_phy_get(struct device_node *np, int index)
{
int ret;
- struct phy_provider *phy_provider;
- struct phy *phy = NULL;
+ struct phy *phy;
struct of_phandle_args args;
lockdep_assert_held(&phy_provider_mutex);
@@ -637,22 +666,7 @@ static struct phy *_of_phy_get(struct device_node *np, int index)
goto out_put_node;
}
- phy_provider = of_phy_provider_lookup(args.np);
- if (IS_ERR(phy_provider) || !try_module_get(phy_provider->owner)) {
- phy = ERR_PTR(-EPROBE_DEFER);
- goto out_put_node;
- }
-
- if (!of_device_is_available(args.np)) {
- dev_warn(phy_provider->dev, "Requested PHY is disabled\n");
- phy = ERR_PTR(-ENODEV);
- goto out_put_module;
- }
-
- phy = phy_provider->of_xlate(phy_provider->dev, &args);
-
-out_put_module:
- module_put(phy_provider->owner);
+ phy = _of_phy_get_with_args(&args);
out_put_node:
of_node_put(args.np);
@@ -1001,6 +1015,41 @@ struct phy *devm_of_phy_get_by_index(struct device *dev, struct device_node *np,
}
EXPORT_SYMBOL_GPL(devm_of_phy_get_by_index);
+/**
+ * phy_get_by_of_node() - lookup and obtain a reference to a phy by device_node
+ * @np: node containing the phy
+ *
+ * Returns the phy associated with the device node or ERR_PTR.
+ */
+struct phy *phy_get_by_of_node(struct device_node *np)
+{
+ struct of_phandle_args args = { .np = np, .args_count = 0 };
+ struct phy *phy;
+
+ if (!np)
+ return ERR_PTR(-EINVAL);
+
+ mutex_lock(&phy_provider_mutex);
+
+ phy = _of_phy_get_with_args(&args);
+
+ if (IS_ERR(phy))
+ goto out_unlock;
+
+ if (!try_module_get(phy->ops->owner)) {
+ phy = ERR_PTR(-EPROBE_DEFER);
+ goto out_unlock;
+ }
+
+ get_device(&phy->dev);
+
+out_unlock:
+ mutex_unlock(&phy_provider_mutex);
+
+ return phy;
+}
+EXPORT_SYMBOL_GPL(phy_get_by_of_node);
+
/**
* phy_create() - create a new phy
* @dev: device that is creating the new phy
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index ea47975e288ae..71c2e16397130 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -284,6 +284,7 @@ struct phy *devm_of_phy_optional_get(struct device *dev, struct device_node *np,
const char *con_id);
struct phy *devm_of_phy_get_by_index(struct device *dev, struct device_node *np,
int index);
+struct phy *phy_get_by_of_node(struct device_node *np);
void of_phy_put(struct phy *phy);
void phy_put(struct device *dev, struct phy *phy);
void devm_phy_put(struct device *dev, struct phy *phy);
@@ -493,6 +494,11 @@ static inline struct phy *devm_of_phy_get_by_index(struct device *dev,
return ERR_PTR(-ENOSYS);
}
+static inline struct phy *phy_get_by_of_node(struct device_node *np)
+{
+ return ERR_PTR(-ENOSYS);
+}
+
static inline void of_phy_put(struct phy *phy)
{
}
--
2.55.0
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v18 2/5] phy: core: Add phy_get_by_of_node()
2026-09-17 11:07 ` [PATCH v18 2/5] phy: core: Add phy_get_by_of_node() Bryan O'Donoghue
@ 2026-09-17 12:29 ` Neil Armstrong
2026-09-17 12:32 ` Bryan O'Donoghue
0 siblings, 1 reply; 13+ messages in thread
From: Neil Armstrong @ 2026-09-17 12:29 UTC (permalink / raw)
To: Bryan O'Donoghue, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Robert Foss, Todor Tomov, Mauro Carvalho Chehab, Konrad Dybcio,
Vladimir Zapolskiy, Bryan O'Donoghue, Loic Poulain,
Vinod Koul, Greg Kroah-Hartman, Kishon Vijay Abraham I,
Felipe Balbi, Manivannan Sadhasivam
Cc: linux-arm-msm, linux-clk, devicetree, linux-kernel, linux-media,
linux-phy, Krzysztof Kozlowski
On 9/17/26 13:07, Bryan O'Donoghue wrote:
> Add new function phy_get_by_of_node() allowing lookup of a phy by
> device_node. Separates existing logic in _of_phy_get() into an internal
> helper method _of_phy_get_with_args() to allow for reuse in new method.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> drivers/phy/phy-core.c | 95 +++++++++++++++++++++++++++++++++++++------------
> include/linux/phy/phy.h | 6 ++++
> 2 files changed, 78 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index 89addd732bff3..490a7cde6d40a 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -606,22 +606,51 @@ int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
> }
> EXPORT_SYMBOL_GPL(phy_validate);
>
> +/**
> + * _of_phy_get_with_args() - lookup and obtain a reference to a phy by of_phandle_args
> + * @args: of_phandle_args to the phy
> + *
> + * Returns the phy from the provider's of_xlate, -ENODEV if disabled,
> + * -EPROBE_DEFER if the provider is not yet registered.
> + */
> +static struct phy *_of_phy_get_with_args(struct of_phandle_args *args)
> +{
> + struct phy *phy;
> + struct phy_provider *phy_provider;
> +
> + lockdep_assert_held(&phy_provider_mutex);
> +
> + phy_provider = of_phy_provider_lookup(args->np);
> + if (IS_ERR(phy_provider) || !try_module_get(phy_provider->owner))
> + return ERR_PTR(-EPROBE_DEFER);
> +
> + if (!of_device_is_available(args->np)) {
> + dev_warn(phy_provider->dev, "Requested PHY is disabled\n");
> + phy = ERR_PTR(-ENODEV);
> + goto out_put_module;
> + }
> +
> + phy = phy_provider->of_xlate(phy_provider->dev, args);
> +
> +out_put_module:
> + module_put(phy_provider->owner);
> +
> + return phy;
> +}
> +
> /**
> * _of_phy_get() - lookup and obtain a reference to a phy by phandle
> * @np: device_node for which to get the phy
> * @index: the index of the phy
> *
> - * Returns the phy associated with the given phandle value,
> - * after getting a refcount to it or -ENODEV if there is no such phy or
> - * -EPROBE_DEFER if there is a phandle to the phy, but the device is
> - * not yet loaded. This function uses of_xlate call back function provided
> - * while registering the phy_provider to find the phy instance.
> + * Returns the phy associated with the given phandle value after getting
> + * a refcount to it; -ENODEV if there is no such phy or the phy is
> + * disabled; -EPROBE_DEFER if the phy provider is not yet available.
> */
Why changing the comment here ? You're not supposed to changes the _of_phy_get() behaviour.
> static struct phy *_of_phy_get(struct device_node *np, int index)
> {
> int ret;
> - struct phy_provider *phy_provider;
> - struct phy *phy = NULL;
> + struct phy *phy;
> struct of_phandle_args args;
>
> lockdep_assert_held(&phy_provider_mutex);
> @@ -637,22 +666,7 @@ static struct phy *_of_phy_get(struct device_node *np, int index)
> goto out_put_node;
> }
>
> - phy_provider = of_phy_provider_lookup(args.np);
> - if (IS_ERR(phy_provider) || !try_module_get(phy_provider->owner)) {
> - phy = ERR_PTR(-EPROBE_DEFER);
> - goto out_put_node;
> - }
> -
> - if (!of_device_is_available(args.np)) {
> - dev_warn(phy_provider->dev, "Requested PHY is disabled\n");
> - phy = ERR_PTR(-ENODEV);
> - goto out_put_module;
> - }
> -
> - phy = phy_provider->of_xlate(phy_provider->dev, &args);
> -
> -out_put_module:
> - module_put(phy_provider->owner);
> + phy = _of_phy_get_with_args(&args);
>
> out_put_node:
> of_node_put(args.np);
> @@ -1001,6 +1015,41 @@ struct phy *devm_of_phy_get_by_index(struct device *dev, struct device_node *np,
> }
> EXPORT_SYMBOL_GPL(devm_of_phy_get_by_index);
>
> +/**
> + * phy_get_by_of_node() - lookup and obtain a reference to a phy by device_node
> + * @np: node containing the phy
> + *
> + * Returns the phy associated with the device node or ERR_PTR.
> + */
> +struct phy *phy_get_by_of_node(struct device_node *np)
> +{
> + struct of_phandle_args args = { .np = np, .args_count = 0 };
> + struct phy *phy;
> +
> + if (!np)
> + return ERR_PTR(-EINVAL);
> +
> + mutex_lock(&phy_provider_mutex);
> +
> + phy = _of_phy_get_with_args(&args);
> +
> + if (IS_ERR(phy))
> + goto out_unlock;
> +
> + if (!try_module_get(phy->ops->owner)) {
> + phy = ERR_PTR(-EPROBE_DEFER);
> + goto out_unlock;
> + }
> +
> + get_device(&phy->dev);
> +
> +out_unlock:
> + mutex_unlock(&phy_provider_mutex);
> +
> + return phy;
> +}
> +EXPORT_SYMBOL_GPL(phy_get_by_of_node);
> +
> /**
> * phy_create() - create a new phy
> * @dev: device that is creating the new phy
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index ea47975e288ae..71c2e16397130 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -284,6 +284,7 @@ struct phy *devm_of_phy_optional_get(struct device *dev, struct device_node *np,
> const char *con_id);
> struct phy *devm_of_phy_get_by_index(struct device *dev, struct device_node *np,
> int index);
> +struct phy *phy_get_by_of_node(struct device_node *np);
> void of_phy_put(struct phy *phy);
> void phy_put(struct device *dev, struct phy *phy);
> void devm_phy_put(struct device *dev, struct phy *phy);
> @@ -493,6 +494,11 @@ static inline struct phy *devm_of_phy_get_by_index(struct device *dev,
> return ERR_PTR(-ENOSYS);
> }
>
> +static inline struct phy *phy_get_by_of_node(struct device_node *np)
> +{
> + return ERR_PTR(-ENOSYS);
> +}
> +
> static inline void of_phy_put(struct phy *phy)
> {
> }
>
Apart that, looks good:
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Thanks,
Neil
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v18 2/5] phy: core: Add phy_get_by_of_node()
2026-09-17 12:29 ` Neil Armstrong
@ 2026-09-17 12:32 ` Bryan O'Donoghue
0 siblings, 0 replies; 13+ messages in thread
From: Bryan O'Donoghue @ 2026-09-17 12:32 UTC (permalink / raw)
To: Neil Armstrong, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Robert Foss,
Todor Tomov, Mauro Carvalho Chehab, Konrad Dybcio,
Vladimir Zapolskiy, Bryan O'Donoghue, Loic Poulain,
Vinod Koul, Greg Kroah-Hartman, Kishon Vijay Abraham I,
Felipe Balbi, Manivannan Sadhasivam
Cc: linux-arm-msm, linux-clk, devicetree, linux-kernel, linux-media,
linux-phy, Krzysztof Kozlowski
On 17/09/2026 13:29, Neil Armstrong wrote:
>> /**
>> * _of_phy_get() - lookup and obtain a reference to a phy by phandle
>> * @np: device_node for which to get the phy
>> * @index: the index of the phy
>> *
>> - * Returns the phy associated with the given phandle value,
>> - * after getting a refcount to it or -ENODEV if there is no such phy or
>> - * -EPROBE_DEFER if there is a phandle to the phy, but the device is
>> - * not yet loaded. This function uses of_xlate call back function
>> provided
>> - * while registering the phy_provider to find the phy instance.
>> + * Returns the phy associated with the given phandle value after getting
>> + * a refcount to it; -ENODEV if there is no such phy or the phy is
>> + * disabled; -EPROBE_DEFER if the phy provider is not yet available.
>> */
>
> Why changing the comment here ? You're not supposed to changes the
> _of_phy_get() behaviour.
I have no real memory of that change probably seemd like a good idea at
the time.
Will drop
---
bod
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v18 3/5] phy: core: Add devm_phy_get_by_of_node()
2026-09-17 11:07 [PATCH v18 0/5] Add phy_get_by_of_node and devm helper Bryan O'Donoghue
2026-09-17 11:07 ` [PATCH v18 1/5] phy: core: Fix race-condition between _of_phy_get() and try_module_get() Bryan O'Donoghue
2026-09-17 11:07 ` [PATCH v18 2/5] phy: core: Add phy_get_by_of_node() Bryan O'Donoghue
@ 2026-09-17 11:07 ` Bryan O'Donoghue
2026-09-17 12:32 ` Neil Armstrong
2026-09-17 11:07 ` [PATCH v18 4/5] media: qcom: camss: Add support for PHY API devices Bryan O'Donoghue
2026-09-17 11:07 ` [PATCH v18 5/5] media: qcom: camss: Use data-lanes starting at 1 for new CSIPHY mode Bryan O'Donoghue
4 siblings, 1 reply; 13+ messages in thread
From: Bryan O'Donoghue @ 2026-09-17 11:07 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Foss, Todor Tomov,
Mauro Carvalho Chehab, Konrad Dybcio, Vladimir Zapolskiy,
Bryan O'Donoghue, Loic Poulain, Vinod Koul, Neil Armstrong,
Greg Kroah-Hartman, Kishon Vijay Abraham I, Felipe Balbi,
Manivannan Sadhasivam
Cc: linux-arm-msm, linux-clk, devicetree, linux-kernel, linux-media,
linux-phy, Bryan O'Donoghue, Krzysztof Kozlowski
Add a devm variant of phy_get_by_of_node() to allow for the familiar
pattern of having devres automatically release resources on the driver's
exit path.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
drivers/phy/phy-core.c | 36 +++++++++++++++++++++++++++++++++++-
include/linux/phy/phy.h | 7 +++++++
2 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 490a7cde6d40a..10e9323566803 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -833,7 +833,7 @@ struct phy *phy_get(struct device *dev, const char *string)
link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
if (!link)
- dev_dbg(dev, "failed to create device link to %s\n",
+ dev_err(dev, "failed to create device link to %s\n",
dev_name(phy->dev.parent));
out_unlock:
@@ -1050,6 +1050,40 @@ struct phy *phy_get_by_of_node(struct device_node *np)
}
EXPORT_SYMBOL_GPL(phy_get_by_of_node);
+/**
+ * devm_phy_get_by_of_node() - devm managed lookup and obtain phy reference by device node
+ * @dev: device requesting the PHY
+ * @np: device_node of the PHY provider
+ *
+ * Returns phy associated with the device_node or ERR_PTR. devres manages
+ * releasing resources.
+ */
+struct phy *devm_phy_get_by_of_node(struct device *dev, struct device_node *np)
+{
+ struct phy **ptr, *phy;
+ struct device_link *link;
+
+ ptr = devres_alloc(devm_phy_release, sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return ERR_PTR(-ENOMEM);
+
+ phy = phy_get_by_of_node(np);
+ if (IS_ERR(phy)) {
+ devres_free(ptr);
+ return phy;
+ }
+
+ *ptr = phy;
+ devres_add(dev, ptr);
+ link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
+ if (!link)
+ dev_dbg(dev, "failed to create device link to %s\n",
+ dev_name(phy->dev.parent));
+
+ return phy;
+}
+EXPORT_SYMBOL_GPL(devm_phy_get_by_of_node);
+
/**
* phy_create() - create a new phy
* @dev: device that is creating the new phy
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 71c2e16397130..14b924a88411f 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -285,6 +285,7 @@ struct phy *devm_of_phy_optional_get(struct device *dev, struct device_node *np,
struct phy *devm_of_phy_get_by_index(struct device *dev, struct device_node *np,
int index);
struct phy *phy_get_by_of_node(struct device_node *np);
+struct phy *devm_phy_get_by_of_node(struct device *dev, struct device_node *np);
void of_phy_put(struct phy *phy);
void phy_put(struct device *dev, struct phy *phy);
void devm_phy_put(struct device *dev, struct phy *phy);
@@ -499,6 +500,12 @@ static inline struct phy *phy_get_by_of_node(struct device_node *np)
return ERR_PTR(-ENOSYS);
}
+static inline struct phy *devm_phy_get_by_of_node(struct device *dev,
+ struct device_node *np)
+{
+ return ERR_PTR(-ENOSYS);
+}
+
static inline void of_phy_put(struct phy *phy)
{
}
--
2.55.0
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v18 3/5] phy: core: Add devm_phy_get_by_of_node()
2026-09-17 11:07 ` [PATCH v18 3/5] phy: core: Add devm_phy_get_by_of_node() Bryan O'Donoghue
@ 2026-09-17 12:32 ` Neil Armstrong
2026-09-17 12:43 ` Bryan O'Donoghue
2026-09-17 13:27 ` Bryan O'Donoghue
0 siblings, 2 replies; 13+ messages in thread
From: Neil Armstrong @ 2026-09-17 12:32 UTC (permalink / raw)
To: Bryan O'Donoghue, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Robert Foss, Todor Tomov, Mauro Carvalho Chehab, Konrad Dybcio,
Vladimir Zapolskiy, Bryan O'Donoghue, Loic Poulain,
Vinod Koul, Greg Kroah-Hartman, Kishon Vijay Abraham I,
Felipe Balbi, Manivannan Sadhasivam
Cc: linux-arm-msm, linux-clk, devicetree, linux-kernel, linux-media,
linux-phy, Krzysztof Kozlowski
On 9/17/26 13:07, Bryan O'Donoghue wrote:
> Add a devm variant of phy_get_by_of_node() to allow for the familiar
> pattern of having devres automatically release resources on the driver's
> exit path.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> drivers/phy/phy-core.c | 36 +++++++++++++++++++++++++++++++++++-
> include/linux/phy/phy.h | 7 +++++++
> 2 files changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index 490a7cde6d40a..10e9323566803 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -833,7 +833,7 @@ struct phy *phy_get(struct device *dev, const char *string)
>
> link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
> if (!link)
> - dev_dbg(dev, "failed to create device link to %s\n",
> + dev_err(dev, "failed to create device link to %s\n",
> dev_name(phy->dev.parent));
This change belongs in another patch.
Neil>
> out_unlock:
> @@ -1050,6 +1050,40 @@ struct phy *phy_get_by_of_node(struct device_node *np)
> }
> EXPORT_SYMBOL_GPL(phy_get_by_of_node);
>
> +/**
> + * devm_phy_get_by_of_node() - devm managed lookup and obtain phy reference by device node
> + * @dev: device requesting the PHY
> + * @np: device_node of the PHY provider
> + *
> + * Returns phy associated with the device_node or ERR_PTR. devres manages
> + * releasing resources.
> + */
> +struct phy *devm_phy_get_by_of_node(struct device *dev, struct device_node *np)
> +{
> + struct phy **ptr, *phy;
> + struct device_link *link;
> +
> + ptr = devres_alloc(devm_phy_release, sizeof(*ptr), GFP_KERNEL);
> + if (!ptr)
> + return ERR_PTR(-ENOMEM);
> +
> + phy = phy_get_by_of_node(np);
> + if (IS_ERR(phy)) {
> + devres_free(ptr);
> + return phy;
> + }
> +
> + *ptr = phy;
> + devres_add(dev, ptr);
> + link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
> + if (!link)
> + dev_dbg(dev, "failed to create device link to %s\n",
> + dev_name(phy->dev.parent));
> +
> + return phy;
> +}
> +EXPORT_SYMBOL_GPL(devm_phy_get_by_of_node);
> +
> /**
> * phy_create() - create a new phy
> * @dev: device that is creating the new phy
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index 71c2e16397130..14b924a88411f 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -285,6 +285,7 @@ struct phy *devm_of_phy_optional_get(struct device *dev, struct device_node *np,
> struct phy *devm_of_phy_get_by_index(struct device *dev, struct device_node *np,
> int index);
> struct phy *phy_get_by_of_node(struct device_node *np);
> +struct phy *devm_phy_get_by_of_node(struct device *dev, struct device_node *np);
> void of_phy_put(struct phy *phy);
> void phy_put(struct device *dev, struct phy *phy);
> void devm_phy_put(struct device *dev, struct phy *phy);
> @@ -499,6 +500,12 @@ static inline struct phy *phy_get_by_of_node(struct device_node *np)
> return ERR_PTR(-ENOSYS);
> }
>
> +static inline struct phy *devm_phy_get_by_of_node(struct device *dev,
> + struct device_node *np)
> +{
> + return ERR_PTR(-ENOSYS);
> +}
> +
> static inline void of_phy_put(struct phy *phy)
> {
> }
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v18 3/5] phy: core: Add devm_phy_get_by_of_node()
2026-09-17 12:32 ` Neil Armstrong
@ 2026-09-17 12:43 ` Bryan O'Donoghue
2026-09-17 13:27 ` Bryan O'Donoghue
1 sibling, 0 replies; 13+ messages in thread
From: Bryan O'Donoghue @ 2026-09-17 12:43 UTC (permalink / raw)
To: Neil Armstrong, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Robert Foss,
Todor Tomov, Mauro Carvalho Chehab, Konrad Dybcio,
Vladimir Zapolskiy, Bryan O'Donoghue, Loic Poulain,
Vinod Koul, Greg Kroah-Hartman, Kishon Vijay Abraham I,
Felipe Balbi, Manivannan Sadhasivam
Cc: linux-arm-msm, linux-clk, devicetree, linux-kernel, linux-media,
linux-phy, Krzysztof Kozlowski
On 17/09/2026 13:32, Neil Armstrong wrote:
>> +++ b/drivers/phy/phy-core.c
>> @@ -833,7 +833,7 @@ struct phy *phy_get(struct device *dev, const char
>> *string)
>> link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
>> if (!link)
>> - dev_dbg(dev, "failed to create device link to %s\n",
>> + dev_err(dev, "failed to create device link to %s\n",
>> dev_name(phy->dev.parent));
>
> This change belongs in another patch.
yeah you're right I messed up my rebase ..
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v18 3/5] phy: core: Add devm_phy_get_by_of_node()
2026-09-17 12:32 ` Neil Armstrong
2026-09-17 12:43 ` Bryan O'Donoghue
@ 2026-09-17 13:27 ` Bryan O'Donoghue
2026-09-17 15:03 ` Bryan O'Donoghue
1 sibling, 1 reply; 13+ messages in thread
From: Bryan O'Donoghue @ 2026-09-17 13:27 UTC (permalink / raw)
To: Neil Armstrong, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Robert Foss,
Todor Tomov, Mauro Carvalho Chehab, Konrad Dybcio,
Vladimir Zapolskiy, Bryan O'Donoghue, Loic Poulain,
Vinod Koul, Greg Kroah-Hartman, Kishon Vijay Abraham I,
Felipe Balbi, Manivannan Sadhasivam
Cc: linux-arm-msm, linux-clk, devicetree, linux-kernel, linux-media,
linux-phy, Krzysztof Kozlowski
On 17/09/2026 13:32, Neil Armstrong wrote:
>> - dev_dbg(dev, "failed to create device link to %s\n",
>> + dev_err(dev, "failed to create device link to %s\n",
>> dev_name(phy->dev.parent));
>
> This change belongs in another patch.
Doh Frank gave me a comment on this and I just changed it without
realising this isn't in my series at all.
I'll drop this altogether
---
bod
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v18 3/5] phy: core: Add devm_phy_get_by_of_node()
2026-09-17 13:27 ` Bryan O'Donoghue
@ 2026-09-17 15:03 ` Bryan O'Donoghue
0 siblings, 0 replies; 13+ messages in thread
From: Bryan O'Donoghue @ 2026-09-17 15:03 UTC (permalink / raw)
To: Bryan O'Donoghue, Neil Armstrong, Bjorn Andersson,
Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Foss, Todor Tomov,
Mauro Carvalho Chehab, Konrad Dybcio, Vladimir Zapolskiy,
Loic Poulain, Vinod Koul, Greg Kroah-Hartman,
Kishon Vijay Abraham I, Felipe Balbi, Manivannan Sadhasivam
Cc: linux-arm-msm, linux-clk, devicetree, linux-kernel, linux-media,
linux-phy, Krzysztof Kozlowski
On 17/09/2026 14:27, Bryan O'Donoghue wrote:
> On 17/09/2026 13:32, Neil Armstrong wrote:
>>> - dev_dbg(dev, "failed to create device link to %s\n",
>>> + dev_err(dev, "failed to create device link to %s\n",
>>> dev_name(phy->dev.parent));
>>
>> This change belongs in another patch.
>
> Doh Frank gave me a comment on this and I just changed it without
> realising this isn't in my series at all.
>
> I'll drop this altogether
>
> ---
> bod
Hmm now I see, there's another device_link_add which does a dev_dbg() on
failure, the feedback was to dev_err() with the part I'm adding below...
+ link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
+ if (!link)
+ dev_dbg(dev, "failed to create device link to %s\n",
+ dev_name(phy->dev.parent));
+
device_link_add() will return NULL when the supplier the PHY is a child
of the consumer so !link is not an error.
---
bod
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v18 4/5] media: qcom: camss: Add support for PHY API devices
2026-09-17 11:07 [PATCH v18 0/5] Add phy_get_by_of_node and devm helper Bryan O'Donoghue
` (2 preceding siblings ...)
2026-09-17 11:07 ` [PATCH v18 3/5] phy: core: Add devm_phy_get_by_of_node() Bryan O'Donoghue
@ 2026-09-17 11:07 ` Bryan O'Donoghue
2026-09-17 11:07 ` [PATCH v18 5/5] media: qcom: camss: Use data-lanes starting at 1 for new CSIPHY mode Bryan O'Donoghue
4 siblings, 0 replies; 13+ messages in thread
From: Bryan O'Donoghue @ 2026-09-17 11:07 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Foss, Todor Tomov,
Mauro Carvalho Chehab, Konrad Dybcio, Vladimir Zapolskiy,
Bryan O'Donoghue, Loic Poulain, Vinod Koul, Neil Armstrong,
Greg Kroah-Hartman, Kishon Vijay Abraham I, Felipe Balbi,
Manivannan Sadhasivam
Cc: linux-arm-msm, linux-clk, devicetree, linux-kernel, linux-media,
linux-phy, Bryan O'Donoghue, Krzysztof Kozlowski
Add the ability to use a PHY pointer which interacts with the standard PHY
API.
In the first instance the code will try to use the new PHY interface. If no
PHYs are present in the DT then the legacy method will be attempted.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
drivers/media/platform/qcom/camss/Kconfig | 1 +
drivers/media/platform/qcom/camss/camss-csiphy.c | 200 +++++++++++++++++++++--
drivers/media/platform/qcom/camss/camss-csiphy.h | 11 +-
drivers/media/platform/qcom/camss/camss.c | 104 ++++++++++--
drivers/media/platform/qcom/camss/camss.h | 1 +
5 files changed, 285 insertions(+), 32 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/Kconfig b/drivers/media/platform/qcom/camss/Kconfig
index 4eda48cb1adf0..1edc5e5a1829e 100644
--- a/drivers/media/platform/qcom/camss/Kconfig
+++ b/drivers/media/platform/qcom/camss/Kconfig
@@ -7,3 +7,4 @@ config VIDEO_QCOM_CAMSS
select VIDEO_V4L2_SUBDEV_API
select VIDEOBUF2_DMA_SG
select V4L2_FWNODE
+ select PHY_QCOM_MIPI_CSI2
diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
index 539ac4888b608..aaed7d0309dd6 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy.c
+++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
@@ -13,6 +13,8 @@
#include <linux/io.h>
#include <linux/kernel.h>
#include <linux/of.h>
+#include <linux/of_graph.h>
+#include <linux/phy/phy.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
#include <media/media-entity.h>
@@ -131,10 +133,10 @@ static u8 csiphy_get_bpp(const struct csiphy_format_info *formats,
}
/*
- * csiphy_set_clock_rates - Calculate and set clock rates on CSIPHY module
+ * csiphy_set_clock_rates_legacy - Calculate and set clock rates on CSIPHY module
* @csiphy: CSIPHY device
*/
-static int csiphy_set_clock_rates(struct csiphy_device *csiphy)
+static int csiphy_set_clock_rates_legacy(struct csiphy_device *csiphy)
{
struct device *dev = csiphy->camss->dev;
s64 link_freq;
@@ -200,7 +202,7 @@ static int csiphy_set_clock_rates(struct csiphy_device *csiphy)
*
* Return 0 on success or a negative error code otherwise
*/
-static int csiphy_set_power(struct v4l2_subdev *sd, int on)
+static int csiphy_set_power_legacy(struct v4l2_subdev *sd, int on)
{
struct csiphy_device *csiphy = v4l2_get_subdevdata(sd);
struct device *dev = csiphy->camss->dev;
@@ -219,7 +221,7 @@ static int csiphy_set_power(struct v4l2_subdev *sd, int on)
return ret;
}
- ret = csiphy_set_clock_rates(csiphy);
+ ret = csiphy_set_clock_rates_legacy(csiphy);
if (ret < 0) {
regulator_bulk_disable(csiphy->num_supplies,
csiphy->supplies);
@@ -254,7 +256,7 @@ static int csiphy_set_power(struct v4l2_subdev *sd, int on)
}
/*
- * csiphy_stream_on - Enable streaming on CSIPHY module
+ * csiphy_stream_on_legacy - Enable streaming on CSIPHY module
* @csiphy: CSIPHY device
*
* Helper function to enable streaming on CSIPHY module.
@@ -262,7 +264,7 @@ static int csiphy_set_power(struct v4l2_subdev *sd, int on)
*
* Return 0 on success or a negative error code otherwise
*/
-static int csiphy_stream_on(struct csiphy_device *csiphy)
+static int csiphy_stream_on_legacy(struct csiphy_device *csiphy)
{
struct csiphy_config *cfg = &csiphy->cfg;
s64 link_freq;
@@ -306,11 +308,98 @@ static int csiphy_stream_on(struct csiphy_device *csiphy)
*
* Helper function to disable streaming on CSIPHY module
*/
-static void csiphy_stream_off(struct csiphy_device *csiphy)
+static void csiphy_stream_off_legacy(struct csiphy_device *csiphy)
{
csiphy->res->hw_ops->lanes_disable(csiphy, &csiphy->cfg);
}
+/*
+ * csiphy_stream_on - Enable streaming on CSIPHY module
+ * @csiphy: CSIPHY device
+ *
+ * Helper function to enable streaming on CSIPHY module.
+ * Main configuration of CSIPHY module is also done here.
+ *
+ * Return 0 on success or a negative error code otherwise
+ */
+static int csiphy_stream_on(struct csiphy_device *csiphy)
+{
+ u8 bpp = csiphy_get_bpp(csiphy->res->formats->formats, csiphy->res->formats->nformats,
+ csiphy->fmt[MSM_CSIPHY_PAD_SINK].code);
+ struct csiphy_lanes_cfg *lncfg = &csiphy->cfg.csi2->lane_cfg;
+ struct phy_configure_opts_mipi_dphy *dphy_cfg;
+ union phy_configure_opts dphy_opts = { 0 };
+ struct device *dev = csiphy->camss->dev;
+ u8 num_lanes = lncfg->num_data;
+ s64 link_freq;
+ int ret;
+
+ dphy_cfg = &dphy_opts.mipi_dphy;
+
+ link_freq = camss_get_link_freq(&csiphy->subdev.entity, bpp, num_lanes);
+
+ if (link_freq <= 0) {
+ dev_err(dev,
+ "Cannot get CSI2 transmitter's link frequency\n");
+ return -EINVAL;
+ }
+
+ ret = phy_mipi_dphy_get_default_config_for_hsclk(link_freq, num_lanes,
+ dphy_cfg);
+ if (ret < 0) {
+ dev_err(dev, "Invalid config for link frequency %lld\n",
+ link_freq);
+ return ret;
+ }
+
+ ret = phy_set_mode(csiphy->phy, PHY_MODE_MIPI_DPHY);
+ if (ret) {
+ dev_err(dev, "Failed to set phy mode\n");
+ return ret;
+ }
+
+ ret = phy_configure(csiphy->phy, &dphy_opts);
+ if (ret) {
+ dev_err(dev, "failed to configure MIPI D-PHY\n");
+ goto error;
+ }
+
+ return phy_power_on(csiphy->phy);
+
+error:
+ return ret;
+}
+
+/*
+ * csiphy_stream_off - Disable streaming on CSIPHY module
+ * @csiphy: CSIPHY device
+ *
+ * Helper function to disable streaming on CSIPHY module
+ */
+static void csiphy_stream_off(struct csiphy_device *csiphy)
+{
+ phy_power_off(csiphy->phy);
+}
+
+/*
+ * csiphy_set_stream - Enable/disable streaming on CSIPHY module
+ * @sd: CSIPHY V4L2 subdevice
+ * @enable: Requested streaming state
+ *
+ * Return 0 on success or a negative error code otherwise
+ */
+static int csiphy_set_stream_legacy(struct v4l2_subdev *sd, int enable)
+{
+ struct csiphy_device *csiphy = v4l2_get_subdevdata(sd);
+ int ret = 0;
+
+ if (enable)
+ ret = csiphy_stream_on_legacy(csiphy);
+ else
+ csiphy_stream_off_legacy(csiphy);
+
+ return ret;
+}
/*
* csiphy_set_stream - Enable/disable streaming on CSIPHY module
@@ -572,16 +661,16 @@ csiphy_match_clock_name(const char *clock_name, const char *format, ...)
}
/*
- * msm_csiphy_subdev_init - Initialize CSIPHY device structure and resources
+ * msm_csiphy_subdev_init_legacy - Initialize CSIPHY device structure and resources
* @csiphy: CSIPHY device
* @res: CSIPHY module resources table
* @id: CSIPHY module id
*
* Return 0 on success or a negative error code otherwise
*/
-int msm_csiphy_subdev_init(struct camss *camss,
- struct csiphy_device *csiphy,
- const struct camss_subdev_resources *res, u8 id)
+int msm_csiphy_subdev_init_legacy(struct camss *camss,
+ struct csiphy_device *csiphy,
+ const struct camss_subdev_resources *res, u8 id)
{
struct device *dev = camss->dev;
struct platform_device *pdev = to_platform_device(dev);
@@ -709,6 +798,54 @@ int msm_csiphy_subdev_init(struct camss *camss,
return ret;
}
+/*
+ * msm_csiphy_subdev_init - Initialize CSIPHY device structure and resources
+ * @camss: CAMSS structure
+ * @port: DT port index
+ *
+ * Return 0 on success or absence of link, negative error code otherwise
+ */
+int msm_csiphy_subdev_init(struct camss *camss, u8 port)
+{
+ const struct camss_subdev_resources *res = &camss->res->csiphy_res[port];
+ struct csiphy_device *csiphy = &camss->csiphy[port];
+ struct device *dev = camss->dev;
+ struct device_node *ep, *remote;
+ int ret;
+
+ ep = of_graph_get_endpoint_by_regs(dev->of_node, port, -1);
+ if (!ep)
+ return 0;
+
+ remote = of_graph_get_remote_port_parent(ep);
+ of_node_put(ep);
+ if (!remote)
+ return 0;
+
+ if (!of_device_is_available(remote)) {
+ of_node_put(remote);
+ return 0;
+ }
+
+ csiphy->phy = devm_phy_get_by_of_node(dev, remote);
+ of_node_put(remote);
+ if (IS_ERR(csiphy->phy)) {
+ ret = PTR_ERR(csiphy->phy);
+ goto done;
+ }
+
+ csiphy->camss = camss;
+ csiphy->id = res->csiphy.id;
+ csiphy->res = &res->csiphy;
+
+ snprintf(csiphy->name, ARRAY_SIZE(csiphy->name), "csi%d", csiphy->id);
+
+ return 0;
+
+done:
+ return ret;
+}
+
/*
* csiphy_link_setup - Setup CSIPHY connections
* @entity: Pointer to media entity structure
@@ -743,8 +880,12 @@ static int csiphy_link_setup(struct media_entity *entity,
return 0;
}
-static const struct v4l2_subdev_core_ops csiphy_core_ops = {
- .s_power = csiphy_set_power,
+static const struct v4l2_subdev_core_ops csiphy_core_ops_legacy = {
+ .s_power = csiphy_set_power_legacy,
+};
+
+static const struct v4l2_subdev_video_ops csiphy_video_ops_legacy = {
+ .s_stream = csiphy_set_stream_legacy,
};
static const struct v4l2_subdev_video_ops csiphy_video_ops = {
@@ -758,8 +899,13 @@ static const struct v4l2_subdev_pad_ops csiphy_pad_ops = {
.set_fmt = csiphy_set_format,
};
+static const struct v4l2_subdev_ops csiphy_v4l2_ops_legacy = {
+ .core = &csiphy_core_ops_legacy,
+ .video = &csiphy_video_ops_legacy,
+ .pad = &csiphy_pad_ops,
+};
+
static const struct v4l2_subdev_ops csiphy_v4l2_ops = {
- .core = &csiphy_core_ops,
.video = &csiphy_video_ops,
.pad = &csiphy_pad_ops,
};
@@ -785,10 +931,15 @@ int msm_csiphy_register_entity(struct csiphy_device *csiphy,
{
struct v4l2_subdev *sd = &csiphy->subdev;
struct media_pad *pads = csiphy->pads;
- struct device *dev = csiphy->camss->dev;
+ struct camss *camss = csiphy->camss;
+ struct device *dev = camss->dev;
int ret;
- v4l2_subdev_init(sd, &csiphy_v4l2_ops);
+ if (camss->legacy_phy)
+ v4l2_subdev_init(sd, &csiphy_v4l2_ops_legacy);
+ else
+ v4l2_subdev_init(sd, &csiphy_v4l2_ops);
+
sd->internal_ops = &csiphy_v4l2_internal_ops;
sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
snprintf(sd->name, ARRAY_SIZE(sd->name), "%s%d",
@@ -816,9 +967,22 @@ int msm_csiphy_register_entity(struct csiphy_device *csiphy,
ret = v4l2_device_register_subdev(v4l2_dev, sd);
if (ret < 0) {
dev_err(dev, "Failed to register subdev: %d\n", ret);
- media_entity_cleanup(&sd->entity);
+ goto media_entity_err;
+ }
+
+ if (!IS_ERR(csiphy->phy)) {
+ ret = phy_init(csiphy->phy);
+ if (ret) {
+ dev_err(dev, "%s init fail %d\n", csiphy->name, ret);
+ v4l2_device_unregister_subdev(sd);
+ goto media_entity_err;
+ }
}
+ return 0;
+
+media_entity_err:
+ media_entity_cleanup(&sd->entity);
return ret;
}
@@ -828,6 +992,8 @@ int msm_csiphy_register_entity(struct csiphy_device *csiphy,
*/
void msm_csiphy_unregister_entity(struct csiphy_device *csiphy)
{
+ if (!IS_ERR(csiphy->phy))
+ phy_exit(csiphy->phy);
v4l2_device_unregister_subdev(&csiphy->subdev);
media_entity_cleanup(&csiphy->subdev.entity);
}
diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.h b/drivers/media/platform/qcom/camss/camss-csiphy.h
index 9d9657b82f748..7a357044b9fdb 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy.h
+++ b/drivers/media/platform/qcom/camss/camss-csiphy.h
@@ -12,6 +12,7 @@
#include <linux/clk.h>
#include <linux/interrupt.h>
+#include <linux/phy/phy.h>
#include <media/media-entity.h>
#include <media/v4l2-device.h>
#include <media/v4l2-mediabus.h>
@@ -97,6 +98,7 @@ struct csiphy_device_regs {
struct csiphy_device {
struct camss *camss;
+ struct phy *phy;
u8 id;
struct v4l2_subdev subdev;
struct media_pad pads[MSM_CSIPHY_PADS_NUM];
@@ -104,6 +106,7 @@ struct csiphy_device {
void __iomem *base_clk_mux;
u32 irq;
char irq_name[30];
+ char name[16];
struct camss_clock *clock;
bool *rate_set;
int nclocks;
@@ -118,9 +121,11 @@ struct csiphy_device {
struct camss_subdev_resources;
-int msm_csiphy_subdev_init(struct camss *camss,
- struct csiphy_device *csiphy,
- const struct camss_subdev_resources *res, u8 id);
+int msm_csiphy_subdev_init_legacy(struct camss *camss,
+ struct csiphy_device *csiphy,
+ const struct camss_subdev_resources *res, u8 id);
+
+int msm_csiphy_subdev_init(struct camss *camss, u8 port);
int msm_csiphy_register_entity(struct csiphy_device *csiphy,
struct v4l2_device *v4l2_dev);
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 2123f6388e3d7..84097d82d99c9 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -4799,8 +4799,43 @@ static int camss_parse_ports(struct camss *camss)
fwnode_graph_for_each_endpoint(fwnode, ep) {
struct camss_async_subdev *csd;
- csd = v4l2_async_nf_add_fwnode_remote(&camss->notifier, ep,
- typeof(*csd));
+ if (!fwnode_device_is_available(ep))
+ continue;
+
+ if (camss->legacy_phy) {
+ csd = v4l2_async_nf_add_fwnode_remote(&camss->notifier, ep,
+ typeof(*csd));
+ } else {
+ struct fwnode_handle *phy_out, *phy_node, *phy_in, *sensor_ep;
+
+ phy_out = fwnode_graph_get_remote_endpoint(ep);
+ if (!phy_out)
+ continue;
+
+ phy_node = fwnode_graph_get_port_parent(phy_out);
+ fwnode_handle_put(phy_out);
+ if (!phy_node)
+ continue;
+
+ phy_in = fwnode_graph_get_endpoint_by_id(phy_node, 0, 0, 0);
+ fwnode_handle_put(phy_node);
+ if (!phy_in)
+ continue;
+
+ sensor_ep = fwnode_graph_get_remote_endpoint(phy_in);
+ fwnode_handle_put(phy_in);
+ if (!sensor_ep)
+ continue;
+
+ csd = v4l2_async_nf_add_fwnode(&camss->notifier, sensor_ep,
+ struct camss_async_subdev);
+ fwnode_handle_put(sensor_ep);
+ if (IS_ERR(csd)) {
+ ret = PTR_ERR(csd);
+ goto err_cleanup;
+ }
+ }
+
if (IS_ERR(csd)) {
ret = PTR_ERR(csd);
goto err_cleanup;
@@ -4819,6 +4854,29 @@ static int camss_parse_ports(struct camss *camss)
return ret;
}
+static void camss_detect_legacy_phy(struct camss *camss)
+{
+ struct device_node *remote;
+ struct device_node *ep;
+
+ camss->legacy_phy = true;
+
+ /* Find first remote-endpoint and determine if its a PHY */
+ for_each_endpoint_of_node(camss->dev->of_node, ep) {
+ remote = of_graph_get_remote_port_parent(ep);
+ if (!remote)
+ continue;
+
+ camss->legacy_phy = !of_node_name_eq(remote, "phy");
+ of_node_put(remote);
+ of_node_put(ep);
+ break;
+ }
+
+ dev_dbg(camss->dev, "legacy phy mode %s\n",
+ camss->legacy_phy ? "true" : "false");
+}
+
/*
* camss_init_subdevices - Initialize subdev structures and resources
* @camss: CAMSS device
@@ -4832,14 +4890,21 @@ static int camss_init_subdevices(struct camss *camss)
unsigned int i;
int ret;
+ camss_detect_legacy_phy(camss);
+
for (i = 0; i < camss->res->csiphy_num; i++) {
- ret = msm_csiphy_subdev_init(camss, &camss->csiphy[i],
- &res->csiphy_res[i],
- res->csiphy_res[i].csiphy.id);
+ if (!camss->legacy_phy) {
+ ret = msm_csiphy_subdev_init(camss, i);
+ } else {
+ ret = msm_csiphy_subdev_init_legacy(camss,
+ &camss->csiphy[i],
+ &res->csiphy_res[i],
+ res->csiphy_res[i].csiphy.id);
+ }
+
if (ret < 0) {
- dev_err(camss->dev,
- "Failed to init csiphy%d sub-device: %d\n",
- i, ret);
+ dev_err(camss->dev, "csiphy %d init fail\n",
+ res->csiphy_res[i].csiphy.id);
return ret;
}
}
@@ -4917,6 +4982,11 @@ inline void camss_link_err(struct camss *camss,
ret);
}
+static inline bool csiphy_enabled(struct camss *camss, struct csiphy_device *c)
+{
+ return camss->legacy_phy || c->phy;
+}
+
/*
* camss_link_entities - Register subdev nodes and create links
* @camss: CAMSS device
@@ -4930,6 +5000,9 @@ static int camss_link_entities(struct camss *camss)
for (i = 0; i < camss->res->csiphy_num; i++) {
for (j = 0; j < camss->res->csid_num; j++) {
+ if (!csiphy_enabled(camss, &camss->csiphy[i]))
+ continue;
+
ret = media_create_pad_link(&camss->csiphy[i].subdev.entity,
MSM_CSIPHY_PAD_SRC,
&camss->csid[j].subdev.entity,
@@ -5056,6 +5129,9 @@ static int camss_register_entities(struct camss *camss)
int ret;
for (i = 0; i < camss->res->csiphy_num; i++) {
+ if (!csiphy_enabled(camss, &camss->csiphy[i]))
+ continue;
+
ret = msm_csiphy_register_entity(&camss->csiphy[i],
&camss->v4l2_dev);
if (ret < 0) {
@@ -5131,8 +5207,10 @@ static int camss_register_entities(struct camss *camss)
i = camss->res->csiphy_num;
err_reg_csiphy:
- for (i--; i >= 0; i--)
- msm_csiphy_unregister_entity(&camss->csiphy[i]);
+ for (i--; i >= 0; i--) {
+ if (csiphy_enabled(camss, &camss->csiphy[i]))
+ msm_csiphy_unregister_entity(&camss->csiphy[i]);
+ }
return ret;
}
@@ -5147,8 +5225,10 @@ static void camss_unregister_entities(struct camss *camss)
{
unsigned int i;
- for (i = 0; i < camss->res->csiphy_num; i++)
- msm_csiphy_unregister_entity(&camss->csiphy[i]);
+ for (i = 0; i < camss->res->csiphy_num; i++) {
+ if (csiphy_enabled(camss, &camss->csiphy[i]))
+ msm_csiphy_unregister_entity(&camss->csiphy[i]);
+ }
if (camss->tpg) {
for (i = 0; i < camss->res->tpg_num; i++)
diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
index 93d691c8ac63b..ba3d51d9c0bf8 100644
--- a/drivers/media/platform/qcom/camss/camss.h
+++ b/drivers/media/platform/qcom/camss/camss.h
@@ -138,6 +138,7 @@ struct camss {
struct device_link *genpd_link;
struct icc_path *icc_path[ICC_SM8250_COUNT];
const struct camss_resources *res;
+ bool legacy_phy;
};
struct camss_camera_interface {
--
2.55.0
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v18 5/5] media: qcom: camss: Use data-lanes starting at 1 for new CSIPHY mode
2026-09-17 11:07 [PATCH v18 0/5] Add phy_get_by_of_node and devm helper Bryan O'Donoghue
` (3 preceding siblings ...)
2026-09-17 11:07 ` [PATCH v18 4/5] media: qcom: camss: Add support for PHY API devices Bryan O'Donoghue
@ 2026-09-17 11:07 ` Bryan O'Donoghue
2026-09-17 13:05 ` Loic Poulain
4 siblings, 1 reply; 13+ messages in thread
From: Bryan O'Donoghue @ 2026-09-17 11:07 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Foss, Todor Tomov,
Mauro Carvalho Chehab, Konrad Dybcio, Vladimir Zapolskiy,
Bryan O'Donoghue, Loic Poulain, Vinod Koul, Neil Armstrong,
Greg Kroah-Hartman, Kishon Vijay Abraham I, Felipe Balbi,
Manivannan Sadhasivam
Cc: linux-arm-msm, linux-clk, devicetree, linux-kernel, linux-media,
linux-phy, Bryan O'Donoghue, Krzysztof Kozlowski
Introducing a dedicated CSIPHY driver community feedback was both to move
to data-lanes starting at index 1 on the PHY side and also to match that
indexing scheme in the CSI decoder - CSID.
CSID consumes the data-lanes property to determine which CSID lanes to
switch on. For indexes starting at 1 we need to amend the logic somewhere.
The PHY side code normalises the input data to register level meanings so,
replicate that logic on the CSID side.
Introduce a simple flag to differentiate between legacy indexing @ 0 and
new indexing @ 1.
Existing bindings are not affected.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
drivers/media/platform/qcom/camss/camss-csiphy.h | 1 +
drivers/media/platform/qcom/camss/camss.c | 15 ++++++++++++---
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.h b/drivers/media/platform/qcom/camss/camss-csiphy.h
index 7a357044b9fdb..8c3319f2ca1cf 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy.h
+++ b/drivers/media/platform/qcom/camss/camss-csiphy.h
@@ -21,6 +21,7 @@
#define MSM_CSIPHY_PAD_SINK 0
#define MSM_CSIPHY_PAD_SRC 1
#define MSM_CSIPHY_PADS_NUM 2
+#define MSM_CSIPHY_MAX_DATA_LANE 4
#define CSIPHY_GRP_ID 1
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 84097d82d99c9..6f58e729eaceb 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -4742,7 +4742,8 @@ static const struct parent_dev_ops vfe_parent_dev_ops = {
*/
static int camss_parse_endpoint_node(struct device *dev,
struct fwnode_handle *ep,
- struct camss_async_subdev *csd)
+ struct camss_async_subdev *csd,
+ u8 lane_base)
{
struct csiphy_lanes_cfg *lncfg = &csd->interface.csi2.lane_cfg;
struct v4l2_mbus_config_mipi_csi2 *mipi_csi2;
@@ -4777,7 +4778,14 @@ static int camss_parse_endpoint_node(struct device *dev,
return -ENOMEM;
for (i = 0; i < lncfg->num_data; i++) {
- lncfg->data[i].pos = mipi_csi2->data_lanes[i];
+ u8 lane = mipi_csi2->data_lanes[i];
+
+ if (lane < lane_base || lane - lane_base >= MSM_CSIPHY_MAX_DATA_LANE) {
+ dev_err(dev, "invalid data-lane %u\n", lane);
+ return -EINVAL;
+ }
+
+ lncfg->data[i].pos = mipi_csi2->data_lanes[i] - lane_base;
lncfg->data[i].pol = mipi_csi2->lane_polarities[i + 1];
}
@@ -4794,6 +4802,7 @@ static int camss_parse_ports(struct camss *camss)
{
struct device *dev = camss->dev;
struct fwnode_handle *fwnode = dev_fwnode(dev), *ep;
+ u8 lane_base = camss->legacy_phy ? 0 : 1;
int ret;
fwnode_graph_for_each_endpoint(fwnode, ep) {
@@ -4841,7 +4850,7 @@ static int camss_parse_ports(struct camss *camss)
goto err_cleanup;
}
- ret = camss_parse_endpoint_node(dev, ep, csd);
+ ret = camss_parse_endpoint_node(dev, ep, csd, lane_base);
if (ret < 0)
goto err_cleanup;
}
--
2.55.0
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v18 5/5] media: qcom: camss: Use data-lanes starting at 1 for new CSIPHY mode
2026-09-17 11:07 ` [PATCH v18 5/5] media: qcom: camss: Use data-lanes starting at 1 for new CSIPHY mode Bryan O'Donoghue
@ 2026-09-17 13:05 ` Loic Poulain
0 siblings, 0 replies; 13+ messages in thread
From: Loic Poulain @ 2026-09-17 13:05 UTC (permalink / raw)
To: Bryan O'Donoghue
Cc: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Foss, Todor Tomov,
Mauro Carvalho Chehab, Konrad Dybcio, Vladimir Zapolskiy,
Bryan O'Donoghue, Vinod Koul, Neil Armstrong,
Greg Kroah-Hartman, Kishon Vijay Abraham I, Felipe Balbi,
Manivannan Sadhasivam, linux-arm-msm, linux-clk, devicetree,
linux-kernel, linux-media, linux-phy, Krzysztof Kozlowski
On Thu, Sep 17, 2026 at 1:08 PM Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> Introducing a dedicated CSIPHY driver community feedback was both to move
> to data-lanes starting at index 1 on the PHY side and also to match that
> indexing scheme in the CSI decoder - CSID.
>
> CSID consumes the data-lanes property to determine which CSID lanes to
> switch on. For indexes starting at 1 we need to amend the logic somewhere.
> The PHY side code normalises the input data to register level meanings so,
> replicate that logic on the CSID side.
>
> Introduce a simple flag to differentiate between legacy indexing @ 0 and
> new indexing @ 1.
>
> Existing bindings are not affected.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
> drivers/media/platform/qcom/camss/camss-csiphy.h | 1 +
> drivers/media/platform/qcom/camss/camss.c | 15 ++++++++++++---
> 2 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.h b/drivers/media/platform/qcom/camss/camss-csiphy.h
> index 7a357044b9fdb..8c3319f2ca1cf 100644
> --- a/drivers/media/platform/qcom/camss/camss-csiphy.h
> +++ b/drivers/media/platform/qcom/camss/camss-csiphy.h
> @@ -21,6 +21,7 @@
> #define MSM_CSIPHY_PAD_SINK 0
> #define MSM_CSIPHY_PAD_SRC 1
> #define MSM_CSIPHY_PADS_NUM 2
> +#define MSM_CSIPHY_MAX_DATA_LANE 4
>
> #define CSIPHY_GRP_ID 1
>
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index 84097d82d99c9..6f58e729eaceb 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -4742,7 +4742,8 @@ static const struct parent_dev_ops vfe_parent_dev_ops = {
> */
> static int camss_parse_endpoint_node(struct device *dev,
> struct fwnode_handle *ep,
> - struct camss_async_subdev *csd)
> + struct camss_async_subdev *csd,
> + u8 lane_base)
> {
> struct csiphy_lanes_cfg *lncfg = &csd->interface.csi2.lane_cfg;
> struct v4l2_mbus_config_mipi_csi2 *mipi_csi2;
> @@ -4777,7 +4778,14 @@ static int camss_parse_endpoint_node(struct device *dev,
> return -ENOMEM;
>
> for (i = 0; i < lncfg->num_data; i++) {
> - lncfg->data[i].pos = mipi_csi2->data_lanes[i];
> + u8 lane = mipi_csi2->data_lanes[i];
> +
> + if (lane < lane_base || lane - lane_base >= MSM_CSIPHY_MAX_DATA_LANE) {
> + dev_err(dev, "invalid data-lane %u\n", lane);
> + return -EINVAL;
> + }
> +
> + lncfg->data[i].pos = mipi_csi2->data_lanes[i] - lane_base;
> lncfg->data[i].pol = mipi_csi2->lane_polarities[i + 1];
> }
>
> @@ -4794,6 +4802,7 @@ static int camss_parse_ports(struct camss *camss)
> {
> struct device *dev = camss->dev;
> struct fwnode_handle *fwnode = dev_fwnode(dev), *ep;
> + u8 lane_base = camss->legacy_phy ? 0 : 1;
> int ret;
>
> fwnode_graph_for_each_endpoint(fwnode, ep) {
> @@ -4841,7 +4850,7 @@ static int camss_parse_ports(struct camss *camss)
> goto err_cleanup;
> }
>
> - ret = camss_parse_endpoint_node(dev, ep, csd);
> + ret = camss_parse_endpoint_node(dev, ep, csd, lane_base);
> if (ret < 0)
> goto err_cleanup;
> }
>
> --
> 2.55.0
>
^ permalink raw reply [flat|nested] 13+ messages in thread