| Message ID | 20220611204651.19947-1-zajec5@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [V3,1/2] mtd: allow getting MTD device associated with a specific DT node | expand |
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
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
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
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 --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);