diff mbox series

[V3,1/2] mtd: allow getting MTD device associated with a specific DT node

Message ID 20220611204651.19947-1-zajec5@gmail.com (mailing list archive)
State New, archived
Headers show
Series [V3,1/2] mtd: allow getting MTD device associated with a specific DT node | expand

Commit Message

Rafał Miłecki June 11, 2022, 8:46 p.m. UTC
From: Rafał Miłecki <rafal@milecki.pl>

MTD subsystem API allows interacting with MTD devices (e.g. reading,
writing, handling bad blocks). So far a random driver could get MTD
device only by its name (get_mtd_device_nm()). This change allows
getting them also by a DT node.

This API is required for drivers handling DT defined MTD partitions in a
specific way (e.g. U-Boot (sub)partition with environment variables).

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V3: First introduction of of_get_mtd_device_by_node()

mtd maintainers: please let know how would you like this patch
processed. Would that be OK for you to Review/Ack it and let it go
through NVMEM tree?
---
 drivers/mtd/mtdcore.c   | 28 ++++++++++++++++++++++++++++
 include/linux/mtd/mtd.h |  1 +
 2 files changed, 29 insertions(+)

Comments

Miquel Raynal June 13, 2022, 2:04 p.m. UTC | #1
Hi Rafał,

zajec5@gmail.com wrote on Sat, 11 Jun 2022 22:46:50 +0200:

> From: Rafał Miłecki <rafal@milecki.pl>
> 
> MTD subsystem API allows interacting with MTD devices (e.g. reading,
> writing, handling bad blocks). So far a random driver could get MTD
> device only by its name (get_mtd_device_nm()). This change allows
> getting them also by a DT node.
> 
> This API is required for drivers handling DT defined MTD partitions in a
> specific way (e.g. U-Boot (sub)partition with environment variables).
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> V3: First introduction of of_get_mtd_device_by_node()
> 
> mtd maintainers: please let know how would you like this patch
> processed. Would that be OK for you to Review/Ack it and let it go
> through NVMEM tree?

Yes

> ---
>  drivers/mtd/mtdcore.c   | 28 ++++++++++++++++++++++++++++
>  include/linux/mtd/mtd.h |  1 +
>  2 files changed, 29 insertions(+)
> 
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 9eb0680db312..7dc214271c85 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -1154,6 +1154,34 @@ int __get_mtd_device(struct mtd_info *mtd)
>  }
>  EXPORT_SYMBOL_GPL(__get_mtd_device);
>  
> +/**
> + * of_get_mtd_device_by_node - obtain an MTD device associated with a given node
> + *
> + * @np: device tree node
> + */
> +struct mtd_info *of_get_mtd_device_by_node(struct device_node *np)

Shall we try to use a more of-agnostic syntax or is it too complex here?

> +{
> +	struct mtd_info *mtd = NULL;
> +	struct mtd_info *tmp;
> +	int err;
> +
> +	mutex_lock(&mtd_table_mutex);
> +
> +	err = -ENODEV;
> +	mtd_for_each_device(tmp) {
> +		if (mtd_get_of_node(tmp) == np) {
> +			mtd = tmp;
> +			err = __get_mtd_device(mtd);
> +			break;
> +		}
> +	}
> +
> +	mutex_unlock(&mtd_table_mutex);
> +
> +	return err ? ERR_PTR(err) : mtd;
> +}
> +EXPORT_SYMBOL_GPL(of_get_mtd_device_by_node);
> +
>  /**
>   *	get_mtd_device_nm - obtain a validated handle for an MTD device by
>   *	device name
> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> index 955aee14b0f7..6fc841ceef31 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -677,6 +677,7 @@ extern int mtd_device_unregister(struct mtd_info *master);
>  extern struct mtd_info *get_mtd_device(struct mtd_info *mtd, int num);
>  extern int __get_mtd_device(struct mtd_info *mtd);
>  extern void __put_mtd_device(struct mtd_info *mtd);
> +extern struct mtd_info *of_get_mtd_device_by_node(struct device_node *np);
>  extern struct mtd_info *get_mtd_device_nm(const char *name);
>  extern void put_mtd_device(struct mtd_info *mtd);
>  


Thanks,
Miquèl
Rafał Miłecki June 13, 2022, 2:15 p.m. UTC | #2
On 13.06.2022 16:04, Miquel Raynal wrote:
>> @@ -1154,6 +1154,34 @@ int __get_mtd_device(struct mtd_info *mtd)
>>   }
>>   EXPORT_SYMBOL_GPL(__get_mtd_device);
>>   
>> +/**
>> + * of_get_mtd_device_by_node - obtain an MTD device associated with a given node
>> + *
>> + * @np: device tree node
>> + */
>> +struct mtd_info *of_get_mtd_device_by_node(struct device_node *np)
> 
> Shall we try to use a more of-agnostic syntax or is it too complex here?

I need some extra hint, please. This is how many similar functions look
like:

$ grep -E -r "(get|find).*_by_node" ./include/*
./include/drm/drm_mipi_dsi.h:struct mipi_dsi_host *of_find_mipi_dsi_host_by_node(struct device_node *node);
./include/drm/drm_mipi_dsi.h:struct mipi_dsi_device *of_find_mipi_dsi_device_by_node(struct device_node *np);
./include/linux/usb/phy.h:extern struct usb_phy *devm_usb_get_phy_by_node(struct device *dev,
./include/linux/usb/phy.h:static inline struct usb_phy *devm_usb_get_phy_by_node(struct device *dev,
./include/linux/extcon.h:struct extcon_dev *extcon_find_edev_by_node(struct device_node *node);
./include/linux/extcon.h:static inline struct extcon_dev *extcon_find_edev_by_node(struct device_node *node)
./include/linux/of_net.h:extern struct net_device *of_find_net_device_by_node(struct device_node *np);
./include/linux/of_net.h:static inline struct net_device *of_find_net_device_by_node(struct device_node *np)
./include/linux/devfreq.h:struct devfreq *devfreq_get_devfreq_by_node(struct device_node *node);
./include/linux/devfreq.h:static inline struct devfreq *devfreq_get_devfreq_by_node(struct device_node *node)
./include/linux/of_platform.h:extern struct platform_device *of_find_device_by_node(struct device_node *np);
./include/linux/of_platform.h:static inline struct platform_device *of_find_device_by_node(struct device_node *np)
./include/linux/backlight.h:struct backlight_device *of_find_backlight_by_node(struct device_node *node);
./include/linux/backlight.h:of_find_backlight_by_node(struct device_node *node)
./include/linux/i2c.h:struct i2c_client *of_find_i2c_device_by_node(struct device_node *node);
./include/linux/i2c.h:struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node);
./include/linux/i2c.h:struct i2c_adapter *of_get_i2c_adapter_by_node(struct device_node *node);
./include/linux/i2c.h:static inline struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)
./include/linux/i2c.h:static inline struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node)
./include/linux/i2c.h:static inline struct i2c_adapter *of_get_i2c_adapter_by_node(struct device_node *node)


>> +{
>> +	struct mtd_info *mtd = NULL;
>> +	struct mtd_info *tmp;
>> +	int err;
>> +
>> +	mutex_lock(&mtd_table_mutex);
>> +
>> +	err = -ENODEV;
>> +	mtd_for_each_device(tmp) {
>> +		if (mtd_get_of_node(tmp) == np) {
>> +			mtd = tmp;
>> +			err = __get_mtd_device(mtd);
>> +			break;
>> +		}
>> +	}
>> +
>> +	mutex_unlock(&mtd_table_mutex);
>> +
>> +	return err ? ERR_PTR(err) : mtd;
>> +}
>> +EXPORT_SYMBOL_GPL(of_get_mtd_device_by_node);
>> +
>>   /**
>>    *	get_mtd_device_nm - obtain a validated handle for an MTD device by
>>    *	device name
Miquel Raynal June 13, 2022, 2:33 p.m. UTC | #3
Hi Rafał,

zajec5@gmail.com wrote on Mon, 13 Jun 2022 16:15:34 +0200:

> On 13.06.2022 16:04, Miquel Raynal wrote:
> >> @@ -1154,6 +1154,34 @@ int __get_mtd_device(struct mtd_info *mtd)
> >>   }
> >>   EXPORT_SYMBOL_GPL(__get_mtd_device);  
> >>   >> +/**  
> >> + * of_get_mtd_device_by_node - obtain an MTD device associated with a given node
> >> + *
> >> + * @np: device tree node
> >> + */
> >> +struct mtd_info *of_get_mtd_device_by_node(struct device_node *np)  
> > 
> > Shall we try to use a more of-agnostic syntax or is it too complex here?  
> 
> I need some extra hint, please. This is how many similar functions look
> like:

I know most implementation today use of_ functions directly but it
seems like there is a global move towards fwnodes now, and I was
wondering if using those instead (which might also apply to other types
of "nodes" than DT ones) could be possible.

But looking into existing implementations, I came across the pwm implem
which features:
- of_pwm_get()
- acpi_pwm_get()

And finally a fwnode_pwm_get() which does:

	if (is_of_node())
		of_pwm_get():
	else if (is_acpi_node())
		acpi_pwm_get();

So actually my suggestion is meaningless. I'm fine with the current
approach.

Acked-by: Miquel Raynal <miquel.raynal@bootlin.com>


> 
> $ grep -E -r "(get|find).*_by_node" ./include/*
> ./include/drm/drm_mipi_dsi.h:struct mipi_dsi_host *of_find_mipi_dsi_host_by_node(struct device_node *node);
> ./include/drm/drm_mipi_dsi.h:struct mipi_dsi_device *of_find_mipi_dsi_device_by_node(struct device_node *np);
> ./include/linux/usb/phy.h:extern struct usb_phy *devm_usb_get_phy_by_node(struct device *dev,
> ./include/linux/usb/phy.h:static inline struct usb_phy *devm_usb_get_phy_by_node(struct device *dev,
> ./include/linux/extcon.h:struct extcon_dev *extcon_find_edev_by_node(struct device_node *node);
> ./include/linux/extcon.h:static inline struct extcon_dev *extcon_find_edev_by_node(struct device_node *node)
> ./include/linux/of_net.h:extern struct net_device *of_find_net_device_by_node(struct device_node *np);
> ./include/linux/of_net.h:static inline struct net_device *of_find_net_device_by_node(struct device_node *np)
> ./include/linux/devfreq.h:struct devfreq *devfreq_get_devfreq_by_node(struct device_node *node);
> ./include/linux/devfreq.h:static inline struct devfreq *devfreq_get_devfreq_by_node(struct device_node *node)
> ./include/linux/of_platform.h:extern struct platform_device *of_find_device_by_node(struct device_node *np);
> ./include/linux/of_platform.h:static inline struct platform_device *of_find_device_by_node(struct device_node *np)
> ./include/linux/backlight.h:struct backlight_device *of_find_backlight_by_node(struct device_node *node);
> ./include/linux/backlight.h:of_find_backlight_by_node(struct device_node *node)
> ./include/linux/i2c.h:struct i2c_client *of_find_i2c_device_by_node(struct device_node *node);
> ./include/linux/i2c.h:struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node);
> ./include/linux/i2c.h:struct i2c_adapter *of_get_i2c_adapter_by_node(struct device_node *node);
> ./include/linux/i2c.h:static inline struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)
> ./include/linux/i2c.h:static inline struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node)
> ./include/linux/i2c.h:static inline struct i2c_adapter *of_get_i2c_adapter_by_node(struct device_node *node)
> 
> 
> >> +{
> >> +	struct mtd_info *mtd = NULL;
> >> +	struct mtd_info *tmp;
> >> +	int err;
> >> +
> >> +	mutex_lock(&mtd_table_mutex);
> >> +
> >> +	err = -ENODEV;
> >> +	mtd_for_each_device(tmp) {
> >> +		if (mtd_get_of_node(tmp) == np) {
> >> +			mtd = tmp;
> >> +			err = __get_mtd_device(mtd);
> >> +			break;
> >> +		}
> >> +	}
> >> +
> >> +	mutex_unlock(&mtd_table_mutex);
> >> +
> >> +	return err ? ERR_PTR(err) : mtd;
> >> +}
> >> +EXPORT_SYMBOL_GPL(of_get_mtd_device_by_node);
> >> +
> >>   /**
> >>    *	get_mtd_device_nm - obtain a validated handle for an MTD device by
> >>    *	device name  
> 


Thanks,
Miquèl
Ahmad Fatoum June 14, 2022, 6:49 a.m. UTC | #4
Hello Rafał,

On 11.06.22 22:46, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> MTD subsystem API allows interacting with MTD devices (e.g. reading,
> writing, handling bad blocks). So far a random driver could get MTD
> device only by its name (get_mtd_device_nm()). This change allows
> getting them also by a DT node.
> 
> This API is required for drivers handling DT defined MTD partitions in a
> specific way (e.g. U-Boot (sub)partition with environment variables).
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> V3: First introduction of of_get_mtd_device_by_node()
> 
> mtd maintainers: please let know how would you like this patch
> processed. Would that be OK for you to Review/Ack it and let it go
> through NVMEM tree?
> ---
>  drivers/mtd/mtdcore.c   | 28 ++++++++++++++++++++++++++++
>  include/linux/mtd/mtd.h |  1 +
>  2 files changed, 29 insertions(+)
> 
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 9eb0680db312..7dc214271c85 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -1154,6 +1154,34 @@ int __get_mtd_device(struct mtd_info *mtd)
>  }
>  EXPORT_SYMBOL_GPL(__get_mtd_device);
>  
> +/**
> + * of_get_mtd_device_by_node - obtain an MTD device associated with a given node
> + *
> + * @np: device tree node
> + */
> +struct mtd_info *of_get_mtd_device_by_node(struct device_node *np)
> +{
> +	struct mtd_info *mtd = NULL;
> +	struct mtd_info *tmp;
> +	int err;
> +
> +	mutex_lock(&mtd_table_mutex);
> +
> +	err = -ENODEV;

Shouldn't this be -EPROBE_DEFER? That way drivers making
use of this function can defer probe until the device
is probed.

> +	mtd_for_each_device(tmp) {
> +		if (mtd_get_of_node(tmp) == np) {
> +			mtd = tmp;
> +			err = __get_mtd_device(mtd);
> +			break;
> +		}
> +	}
> +
> +	mutex_unlock(&mtd_table_mutex);
> +
> +	return err ? ERR_PTR(err) : mtd;
> +}
> +EXPORT_SYMBOL_GPL(of_get_mtd_device_by_node);
> +
>  /**
>   *	get_mtd_device_nm - obtain a validated handle for an MTD device by
>   *	device name
> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> index 955aee14b0f7..6fc841ceef31 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -677,6 +677,7 @@ extern int mtd_device_unregister(struct mtd_info *master);
>  extern struct mtd_info *get_mtd_device(struct mtd_info *mtd, int num);
>  extern int __get_mtd_device(struct mtd_info *mtd);
>  extern void __put_mtd_device(struct mtd_info *mtd);
> +extern struct mtd_info *of_get_mtd_device_by_node(struct device_node *np);
>  extern struct mtd_info *get_mtd_device_nm(const char *name);
>  extern void put_mtd_device(struct mtd_info *mtd);
>
diff mbox series

Patch

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 9eb0680db312..7dc214271c85 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -1154,6 +1154,34 @@  int __get_mtd_device(struct mtd_info *mtd)
 }
 EXPORT_SYMBOL_GPL(__get_mtd_device);
 
+/**
+ * of_get_mtd_device_by_node - obtain an MTD device associated with a given node
+ *
+ * @np: device tree node
+ */
+struct mtd_info *of_get_mtd_device_by_node(struct device_node *np)
+{
+	struct mtd_info *mtd = NULL;
+	struct mtd_info *tmp;
+	int err;
+
+	mutex_lock(&mtd_table_mutex);
+
+	err = -ENODEV;
+	mtd_for_each_device(tmp) {
+		if (mtd_get_of_node(tmp) == np) {
+			mtd = tmp;
+			err = __get_mtd_device(mtd);
+			break;
+		}
+	}
+
+	mutex_unlock(&mtd_table_mutex);
+
+	return err ? ERR_PTR(err) : mtd;
+}
+EXPORT_SYMBOL_GPL(of_get_mtd_device_by_node);
+
 /**
  *	get_mtd_device_nm - obtain a validated handle for an MTD device by
  *	device name
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 955aee14b0f7..6fc841ceef31 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -677,6 +677,7 @@  extern int mtd_device_unregister(struct mtd_info *master);
 extern struct mtd_info *get_mtd_device(struct mtd_info *mtd, int num);
 extern int __get_mtd_device(struct mtd_info *mtd);
 extern void __put_mtd_device(struct mtd_info *mtd);
+extern struct mtd_info *of_get_mtd_device_by_node(struct device_node *np);
 extern struct mtd_info *get_mtd_device_nm(const char *name);
 extern void put_mtd_device(struct mtd_info *mtd);