[v2,03/21] of: Rename of_modalias_node()
Commit Message
This helper does not produce a real modalias, but tries to get the
"product" compatible part of the "vendor,product" compatibles only. It
is far from creating a purely useful modalias string and does not seem
to be used like that directly anyway, so let's try to give this helper a
more meaningful name before moving there a real modalias helper (already
existing under of/device.c).
Also update the various documentations to refer to the strings as
"aliases" rather than "modaliases" which has a real meaning in the Linux
kernel.
There is no functional change.
Cc: Rafael J. Wysocki <rafael@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Sebastian Reichel <sre@kernel.org>
Cc: Wolfram Sang <wsa@kernel.org>
Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/acpi/bus.c | 7 ++++---
drivers/gpu/drm/drm_mipi_dsi.c | 2 +-
drivers/hsi/hsi_core.c | 2 +-
drivers/i2c/busses/i2c-powermac.c | 2 +-
drivers/i2c/i2c-core-of.c | 2 +-
drivers/of/base.c | 15 ++++++++-------
drivers/spi/spi.c | 4 ++--
include/linux/of.h | 2 +-
8 files changed, 19 insertions(+), 17 deletions(-)
Comments
On Tue, Mar 07, 2023 at 05:53:41PM +0100, Miquel Raynal wrote:
> This helper does not produce a real modalias, but tries to get the
> "product" compatible part of the "vendor,product" compatibles only. It
> is far from creating a purely useful modalias string and does not seem
> to be used like that directly anyway, so let's try to give this helper a
> more meaningful name before moving there a real modalias helper (already
> existing under of/device.c).
>
> Also update the various documentations to refer to the strings as
> "aliases" rather than "modaliases" which has a real meaning in the Linux
> kernel.
>
> There is no functional change.
>
> Cc: Rafael J. Wysocki <rafael@kernel.org>
> Cc: Len Brown <lenb@kernel.org>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: Wolfram Sang <wsa@kernel.org>
> Cc: Mark Brown <broonie@kernel.org>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> drivers/acpi/bus.c | 7 ++++---
> drivers/gpu/drm/drm_mipi_dsi.c | 2 +-
> drivers/hsi/hsi_core.c | 2 +-
These should not have been using this function. The matching on just the
product was a relic from I2C and SPI which we don't want to propogate.
No clue why ACPI needed it...
If you respin or want to fixup while applying, can you add a kerneldoc
comment to not add new users of the function. Not that anyone will
follow that... :(
Reviewed-by: Rob Herring <robh@kernel.org>
> drivers/i2c/busses/i2c-powermac.c | 2 +-
> drivers/i2c/i2c-core-of.c | 2 +-
> drivers/of/base.c | 15 ++++++++-------
> drivers/spi/spi.c | 4 ++--
> include/linux/of.h | 2 +-
> 8 files changed, 19 insertions(+), 17 deletions(-)
Hi Rob,
robh@kernel.org wrote on Tue, 7 Mar 2023 18:19:03 -0600:
> On Tue, Mar 07, 2023 at 05:53:41PM +0100, Miquel Raynal wrote:
> > This helper does not produce a real modalias, but tries to get the
> > "product" compatible part of the "vendor,product" compatibles only. It
> > is far from creating a purely useful modalias string and does not seem
> > to be used like that directly anyway, so let's try to give this helper a
> > more meaningful name before moving there a real modalias helper (already
> > existing under of/device.c).
> >
> > Also update the various documentations to refer to the strings as
> > "aliases" rather than "modaliases" which has a real meaning in the Linux
> > kernel.
> >
> > There is no functional change.
> >
> > Cc: Rafael J. Wysocki <rafael@kernel.org>
> > Cc: Len Brown <lenb@kernel.org>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Maxime Ripard <mripard@kernel.org>
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: Sebastian Reichel <sre@kernel.org>
> > Cc: Wolfram Sang <wsa@kernel.org>
> > Cc: Mark Brown <broonie@kernel.org>
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> > drivers/acpi/bus.c | 7 ++++---
> > drivers/gpu/drm/drm_mipi_dsi.c | 2 +-
> > drivers/hsi/hsi_core.c | 2 +-
>
> These should not have been using this function. The matching on just the
> product was a relic from I2C and SPI which we don't want to propogate.
> No clue why ACPI needed it...
>
> If you respin or want to fixup while applying, can you add a kerneldoc
> comment to not add new users of the function. Not that anyone will
> follow that... :(
I extensively scratched my forehead while trying to understand the use
of this function. I've added this to my v3:
* It does this by stripping the manufacturer prefix (as delimited by a ',')
* from the first entry in the compatible list property.
*
+ * Note: The matching on just the "product" side of the compatible is a relic
+ * from I2C and SPI. Please do not add any new user.
+ *
* Return: This routine returns 0 on success, <0 on failure.
*/
> Reviewed-by: Rob Herring <robh@kernel.org>
>
> > drivers/i2c/busses/i2c-powermac.c | 2 +-
> > drivers/i2c/i2c-core-of.c | 2 +-
> > drivers/of/base.c | 15 ++++++++-------
> > drivers/spi/spi.c | 4 ++--
> > include/linux/of.h | 2 +-
> > 8 files changed, 19 insertions(+), 17 deletions(-)
Thanks,
Miquèl
Hi,
On Tue, Mar 07, 2023 at 06:19:03PM -0600, Rob Herring wrote:
> On Tue, Mar 07, 2023 at 05:53:41PM +0100, Miquel Raynal wrote:
> > This helper does not produce a real modalias, but tries to get the
> > "product" compatible part of the "vendor,product" compatibles only. It
> > is far from creating a purely useful modalias string and does not seem
> > to be used like that directly anyway, so let's try to give this helper a
> > more meaningful name before moving there a real modalias helper (already
> > existing under of/device.c).
> >
> > Also update the various documentations to refer to the strings as
> > "aliases" rather than "modaliases" which has a real meaning in the Linux
> > kernel.
> >
> > There is no functional change.
> >
> > Cc: Rafael J. Wysocki <rafael@kernel.org>
> > Cc: Len Brown <lenb@kernel.org>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Maxime Ripard <mripard@kernel.org>
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: Sebastian Reichel <sre@kernel.org>
> > Cc: Wolfram Sang <wsa@kernel.org>
> > Cc: Mark Brown <broonie@kernel.org>
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> > drivers/acpi/bus.c | 7 ++++---
> > drivers/gpu/drm/drm_mipi_dsi.c | 2 +-
> > drivers/hsi/hsi_core.c | 2 +-
>
> These should not have been using this function. The matching on just the
> product was a relic from I2C and SPI which we don't want to propogate.
> No clue why ACPI needed it...
>
> If you respin or want to fixup while applying, can you add a kerneldoc
> comment to not add new users of the function. Not that anyone will
> follow that... :(
>
> Reviewed-by: Rob Herring <robh@kernel.org>
I just checked and HSI is not using the data for matching. Instead
it uses the returned data to set the device name. Matching happens
using the full compatible.
FWIW the MIPI HSI standard never became a big thing, so we have only
one HSI DT driver upstream and that is the Nokia N900 modem driver.
Anyways:
Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com> # for HSI
-- Sebastian
>
> > drivers/i2c/busses/i2c-powermac.c | 2 +-
> > drivers/i2c/i2c-core-of.c | 2 +-
> > drivers/of/base.c | 15 ++++++++-------
> > drivers/spi/spi.c | 4 ++--
> > include/linux/of.h | 2 +-
> > 8 files changed, 19 insertions(+), 17 deletions(-)
On Wed, Mar 8, 2023 at 4:06 PM Sebastian Reichel
<sebastian.reichel@collabora.com> wrote:
>
> Hi,
>
> On Tue, Mar 07, 2023 at 06:19:03PM -0600, Rob Herring wrote:
> > On Tue, Mar 07, 2023 at 05:53:41PM +0100, Miquel Raynal wrote:
> > > This helper does not produce a real modalias, but tries to get the
> > > "product" compatible part of the "vendor,product" compatibles only. It
> > > is far from creating a purely useful modalias string and does not seem
> > > to be used like that directly anyway, so let's try to give this helper a
> > > more meaningful name before moving there a real modalias helper (already
> > > existing under of/device.c).
> > >
> > > Also update the various documentations to refer to the strings as
> > > "aliases" rather than "modaliases" which has a real meaning in the Linux
> > > kernel.
> > >
> > > There is no functional change.
> > >
> > > Cc: Rafael J. Wysocki <rafael@kernel.org>
> > > Cc: Len Brown <lenb@kernel.org>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Cc: Maxime Ripard <mripard@kernel.org>
> > > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > > Cc: Sebastian Reichel <sre@kernel.org>
> > > Cc: Wolfram Sang <wsa@kernel.org>
> > > Cc: Mark Brown <broonie@kernel.org>
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > ---
> > > drivers/acpi/bus.c | 7 ++++---
> > > drivers/gpu/drm/drm_mipi_dsi.c | 2 +-
> > > drivers/hsi/hsi_core.c | 2 +-
> >
> > These should not have been using this function. The matching on just the
> > product was a relic from I2C and SPI which we don't want to propogate.
> > No clue why ACPI needed it...
> >
> > If you respin or want to fixup while applying, can you add a kerneldoc
> > comment to not add new users of the function. Not that anyone will
> > follow that... :(
> >
> > Reviewed-by: Rob Herring <robh@kernel.org>
>
> I just checked and HSI is not using the data for matching. Instead
> it uses the returned data to set the device name. Matching happens
> using the full compatible.
>
> FWIW the MIPI HSI standard never became a big thing, so we have only
> one HSI DT driver upstream and that is the Nokia N900 modem driver.
Can we add a patch in removing the call then.
I'm pretty sure MIPI stands for Must Invent Peripheral Interfaces.
Rob
@@ -817,9 +817,10 @@ static bool acpi_of_modalias(struct acpi_device *adev,
* @modalias: Pointer to buffer that modalias value will be copied into
* @len: Length of modalias buffer
*
- * This is a counterpart of of_modalias_node() for struct acpi_device objects.
- * If there is a compatible string for @adev, it will be copied to @modalias
- * with the vendor prefix stripped; otherwise, @default_id will be used.
+ * This is a counterpart of of_alias_from_compatible() for struct acpi_device
+ * objects. If there is a compatible string for @adev, it will be copied to
+ * @modalias with the vendor prefix stripped; otherwise, @default_id will be
+ * used.
*/
void acpi_set_modalias(struct acpi_device *adev, const char *default_id,
char *modalias, size_t len)
@@ -160,7 +160,7 @@ of_mipi_dsi_device_add(struct mipi_dsi_host *host, struct device_node *node)
int ret;
u32 reg;
- if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
+ if (of_alias_from_compatible(node, info.type, sizeof(info.type)) < 0) {
drm_err(host, "modalias failure on %pOF\n", node);
return ERR_PTR(-EINVAL);
}
@@ -207,7 +207,7 @@ static void hsi_add_client_from_dt(struct hsi_port *port,
if (!cl)
return;
- err = of_modalias_node(client, name, sizeof(name));
+ err = of_alias_from_compatible(client, name, sizeof(name));
if (err)
goto err;
@@ -284,7 +284,7 @@ static bool i2c_powermac_get_type(struct i2c_adapter *adap,
*/
/* First try proper modalias */
- if (of_modalias_node(node, tmp, sizeof(tmp)) >= 0) {
+ if (of_alias_from_compatible(node, tmp, sizeof(tmp)) >= 0) {
snprintf(type, type_size, "MAC,%s", tmp);
return true;
}
@@ -27,7 +27,7 @@ int of_i2c_get_board_info(struct device *dev, struct device_node *node,
memset(info, 0, sizeof(*info));
- if (of_modalias_node(node, info->type, sizeof(info->type)) < 0) {
+ if (of_alias_from_compatible(node, info->type, sizeof(info->type)) < 0) {
dev_err(dev, "of_i2c: modalias failure on %pOF\n", node);
return -EINVAL;
}
@@ -1208,19 +1208,20 @@ struct device_node *of_find_matching_node_and_match(struct device_node *from,
EXPORT_SYMBOL(of_find_matching_node_and_match);
/**
- * of_modalias_node - Lookup appropriate modalias for a device node
+ * of_alias_from_compatible - Lookup appropriate alias for a device node
+ * depending on compatible
* @node: pointer to a device tree node
- * @modalias: Pointer to buffer that modalias value will be copied into
- * @len: Length of modalias value
+ * @modalias: Pointer to buffer that alias value will be copied into
+ * @len: Length of alias value
*
* Based on the value of the compatible property, this routine will attempt
- * to choose an appropriate modalias value for a particular device tree node.
+ * to choose an appropriate alias value for a particular device tree node.
* It does this by stripping the manufacturer prefix (as delimited by a ',')
* from the first entry in the compatible list property.
*
* Return: This routine returns 0 on success, <0 on failure.
*/
-int of_modalias_node(struct device_node *node, char *modalias, int len)
+int of_alias_from_compatible(struct device_node *node, char *alias, int len)
{
const char *compatible, *p;
int cplen;
@@ -1229,10 +1230,10 @@ int of_modalias_node(struct device_node *node, char *modalias, int len)
if (!compatible || strlen(compatible) > cplen)
return -ENODEV;
p = strchr(compatible, ',');
- strscpy(modalias, p ? p + 1 : compatible, len);
+ strscpy(alias, p ? p + 1 : compatible, len);
return 0;
}
-EXPORT_SYMBOL_GPL(of_modalias_node);
+EXPORT_SYMBOL_GPL(of_alias_from_compatible);
/**
* of_find_node_by_phandle - Find a node given a phandle
@@ -2333,8 +2333,8 @@ of_register_spi_device(struct spi_controller *ctlr, struct device_node *nc)
}
/* Select device driver */
- rc = of_modalias_node(nc, spi->modalias,
- sizeof(spi->modalias));
+ rc = of_alias_from_compatible(nc, spi->modalias,
+ sizeof(spi->modalias));
if (rc < 0) {
dev_err(&ctlr->dev, "cannot find modalias for %pOF\n", nc);
goto err_out;
@@ -362,7 +362,7 @@ extern int of_n_addr_cells(struct device_node *np);
extern int of_n_size_cells(struct device_node *np);
extern const struct of_device_id *of_match_node(
const struct of_device_id *matches, const struct device_node *node);
-extern int of_modalias_node(struct device_node *node, char *modalias, int len);
+extern int of_alias_from_compatible(struct device_node *node, char *alias, int len);
extern void of_print_phandle_args(const char *msg, const struct of_phandle_args *args);
extern int __of_parse_phandle_with_args(const struct device_node *np,
const char *list_name, const char *cells_name, int cell_count,