| Message ID | 20211203212358.31444-2-anand.ashok.dumbre@xilinx.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | Add Xilinx AMS Driver | expand |
On Fri, 3 Dec 2021 21:23:54 +0000 Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com> wrote: > This patch introduces a new helper routine - fwnode_iomap(), which > allows to map the memory mapped IO for a given device node. > > This implementation does not cover the ACPI case and may be expanded > in the future. The main purpose here is to be able to develop resource > provider agnostic drivers. > > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> This is one of those corners of the kernel where I'm not sure whose Acks etc I should be looking for. Maintainers would put it firmly in Greg's territory but seems we've been flexible around this particular file at times. History seems to suggest maybe Greg or Rafael? If either of you have time to sanity check this that would be great! Thanks, Jonathan > --- > drivers/base/property.c | 16 ++++++++++++++++ > include/linux/property.h | 2 ++ > 2 files changed, 18 insertions(+) > > diff --git a/drivers/base/property.c b/drivers/base/property.c > index f1f35b48ab8b..ed4470410030 100644 > --- a/drivers/base/property.c > +++ b/drivers/base/property.c > @@ -958,6 +958,22 @@ int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index) > } > EXPORT_SYMBOL(fwnode_irq_get); > > +/** > + * fwnode_iomap - Maps the memory mapped IO for a given fwnode > + * @fwnode: Pointer to the firmware node > + * @index: Index of the IO range > + * > + * Returns a pointer to the mapped memory. > + */ > +void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int index) > +{ > + if (IS_ENABLED(CONFIG_OF_ADDRESS) && is_of_node(fwnode)) > + return of_iomap(to_of_node(fwnode), index); > + > + return NULL; > +} > +EXPORT_SYMBOL(fwnode_iomap); > + > /** > * fwnode_graph_get_next_endpoint - Get next endpoint firmware node > * @fwnode: Pointer to the parent firmware node > diff --git a/include/linux/property.h b/include/linux/property.h > index 88fa726a76df..6670d5a1ec2a 100644 > --- a/include/linux/property.h > +++ b/include/linux/property.h > @@ -122,6 +122,8 @@ void fwnode_handle_put(struct fwnode_handle *fwnode); > > int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index); > > +void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int index); > + > unsigned int device_get_child_node_count(struct device *dev); > > static inline bool device_property_read_bool(struct device *dev,
On Fri, Dec 3, 2021 at 10:24 PM Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com> wrote: > > This patch introduces a new helper routine - fwnode_iomap(), which > allows to map the memory mapped IO for a given device node. > > This implementation does not cover the ACPI case and may be expanded > in the future. The main purpose here is to be able to develop resource > provider agnostic drivers. > > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/base/property.c | 16 ++++++++++++++++ > include/linux/property.h | 2 ++ > 2 files changed, 18 insertions(+) > > diff --git a/drivers/base/property.c b/drivers/base/property.c > index f1f35b48ab8b..ed4470410030 100644 > --- a/drivers/base/property.c > +++ b/drivers/base/property.c > @@ -958,6 +958,22 @@ int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index) > } > EXPORT_SYMBOL(fwnode_irq_get); > > +/** > + * fwnode_iomap - Maps the memory mapped IO for a given fwnode > + * @fwnode: Pointer to the firmware node > + * @index: Index of the IO range > + * > + * Returns a pointer to the mapped memory. > + */ > +void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int index) > +{ > + if (IS_ENABLED(CONFIG_OF_ADDRESS) && is_of_node(fwnode)) > + return of_iomap(to_of_node(fwnode), index); > + > + return NULL; > +} > +EXPORT_SYMBOL(fwnode_iomap); So why is this EXPORT_SYMBOL() and not EXPORT_SYMBOL_GPL()? Other than this I'm not an OF_ expert, but I trust Andy, so with the above addressed: Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > + > /** > * fwnode_graph_get_next_endpoint - Get next endpoint firmware node > * @fwnode: Pointer to the parent firmware node > diff --git a/include/linux/property.h b/include/linux/property.h > index 88fa726a76df..6670d5a1ec2a 100644 > --- a/include/linux/property.h > +++ b/include/linux/property.h > @@ -122,6 +122,8 @@ void fwnode_handle_put(struct fwnode_handle *fwnode); > > int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index); > > +void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int index); > + > unsigned int device_get_child_node_count(struct device *dev); > > static inline bool device_property_read_bool(struct device *dev, > -- > 2.17.1 >
On Fri, 17 Dec 2021 18:52:00 +0100 "Rafael J. Wysocki" <rafael@kernel.org> wrote: > On Fri, Dec 3, 2021 at 10:24 PM Anand Ashok Dumbre > <anand.ashok.dumbre@xilinx.com> wrote: > > > > This patch introduces a new helper routine - fwnode_iomap(), which > > allows to map the memory mapped IO for a given device node. > > > > This implementation does not cover the ACPI case and may be expanded > > in the future. The main purpose here is to be able to develop resource > > provider agnostic drivers. > > > > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Signed-off-by: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com> > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > --- > > drivers/base/property.c | 16 ++++++++++++++++ > > include/linux/property.h | 2 ++ > > 2 files changed, 18 insertions(+) > > > > diff --git a/drivers/base/property.c b/drivers/base/property.c > > index f1f35b48ab8b..ed4470410030 100644 > > --- a/drivers/base/property.c > > +++ b/drivers/base/property.c > > @@ -958,6 +958,22 @@ int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index) > > } > > EXPORT_SYMBOL(fwnode_irq_get); > > > > +/** > > + * fwnode_iomap - Maps the memory mapped IO for a given fwnode > > + * @fwnode: Pointer to the firmware node > > + * @index: Index of the IO range > > + * > > + * Returns a pointer to the mapped memory. > > + */ > > +void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int index) > > +{ > > + if (IS_ENABLED(CONFIG_OF_ADDRESS) && is_of_node(fwnode)) > > + return of_iomap(to_of_node(fwnode), index); > > + > > + return NULL; > > +} > > +EXPORT_SYMBOL(fwnode_iomap); > > So why is this EXPORT_SYMBOL() and not EXPORT_SYMBOL_GPL()? Good question. I'm guessing this is because of_iomap is EXPORT_SYMBOL() and we don't want to discourage use of this function in preference to it. Series applied to the togreg branch of iio.git but initially pushed out as testing to let 0-day see if it can find anything we broke. Note I can still rebase if people would prefer _GPL() so feel free to discuss further. +CC Rob Herring and DT list. Thanks, Jonathan > > Other than this I'm not an OF_ expert, but I trust Andy, so with the > above addressed: > > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > + > > /** > > * fwnode_graph_get_next_endpoint - Get next endpoint firmware node > > * @fwnode: Pointer to the parent firmware node > > diff --git a/include/linux/property.h b/include/linux/property.h > > index 88fa726a76df..6670d5a1ec2a 100644 > > --- a/include/linux/property.h > > +++ b/include/linux/property.h > > @@ -122,6 +122,8 @@ void fwnode_handle_put(struct fwnode_handle *fwnode); > > > > int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index); > > > > +void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int index); > > + > > unsigned int device_get_child_node_count(struct device *dev); > > > > static inline bool device_property_read_bool(struct device *dev, > > -- > > 2.17.1 > >
On Sat, 18 Dec 2021 10:38:24 +0000 Jonathan Cameron <jic23@kernel.org> wrote: > On Fri, 17 Dec 2021 18:52:00 +0100 > "Rafael J. Wysocki" <rafael@kernel.org> wrote: > > > On Fri, Dec 3, 2021 at 10:24 PM Anand Ashok Dumbre > > <anand.ashok.dumbre@xilinx.com> wrote: > > > > > > This patch introduces a new helper routine - fwnode_iomap(), which > > > allows to map the memory mapped IO for a given device node. > > > > > > This implementation does not cover the ACPI case and may be expanded > > > in the future. The main purpose here is to be able to develop resource > > > provider agnostic drivers. > > > > > > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > Signed-off-by: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com> > > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > --- > > > drivers/base/property.c | 16 ++++++++++++++++ > > > include/linux/property.h | 2 ++ > > > 2 files changed, 18 insertions(+) > > > > > > diff --git a/drivers/base/property.c b/drivers/base/property.c > > > index f1f35b48ab8b..ed4470410030 100644 > > > --- a/drivers/base/property.c > > > +++ b/drivers/base/property.c > > > @@ -958,6 +958,22 @@ int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index) > > > } > > > EXPORT_SYMBOL(fwnode_irq_get); > > > > > > +/** > > > + * fwnode_iomap - Maps the memory mapped IO for a given fwnode > > > + * @fwnode: Pointer to the firmware node > > > + * @index: Index of the IO range > > > + * > > > + * Returns a pointer to the mapped memory. > > > + */ > > > +void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int index) > > > +{ > > > + if (IS_ENABLED(CONFIG_OF_ADDRESS) && is_of_node(fwnode)) > > > + return of_iomap(to_of_node(fwnode), index); > > > + > > > + return NULL; > > > +} > > > +EXPORT_SYMBOL(fwnode_iomap); > > > > So why is this EXPORT_SYMBOL() and not EXPORT_SYMBOL_GPL()? > > Good question. I'm guessing this is because of_iomap is EXPORT_SYMBOL() > and we don't want to discourage use of this function in preference to it. > > Series applied to the togreg branch of iio.git but initially pushed out > as testing to let 0-day see if it can find anything we broke. Note that I wrongly picked up the dtsi change so now dropped that. Also a trivial indentation fix that 0-day found and I've rolled into the original patch. Thanks, Jonathan > > Note I can still rebase if people would prefer _GPL() so feel free to > discuss further. +CC Rob Herring and DT list. > > Thanks, > > Jonathan > > > > > > Other than this I'm not an OF_ expert, but I trust Andy, so with the > > above addressed: > > > > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > > + > > > /** > > > * fwnode_graph_get_next_endpoint - Get next endpoint firmware node > > > * @fwnode: Pointer to the parent firmware node > > > diff --git a/include/linux/property.h b/include/linux/property.h > > > index 88fa726a76df..6670d5a1ec2a 100644 > > > --- a/include/linux/property.h > > > +++ b/include/linux/property.h > > > @@ -122,6 +122,8 @@ void fwnode_handle_put(struct fwnode_handle *fwnode); > > > > > > int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index); > > > > > > +void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int index); > > > + > > > unsigned int device_get_child_node_count(struct device *dev); > > > > > > static inline bool device_property_read_bool(struct device *dev, > > > -- > > > 2.17.1 > > > >
diff --git a/drivers/base/property.c b/drivers/base/property.c index f1f35b48ab8b..ed4470410030 100644 --- a/drivers/base/property.c +++ b/drivers/base/property.c @@ -958,6 +958,22 @@ int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index) } EXPORT_SYMBOL(fwnode_irq_get); +/** + * fwnode_iomap - Maps the memory mapped IO for a given fwnode + * @fwnode: Pointer to the firmware node + * @index: Index of the IO range + * + * Returns a pointer to the mapped memory. + */ +void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int index) +{ + if (IS_ENABLED(CONFIG_OF_ADDRESS) && is_of_node(fwnode)) + return of_iomap(to_of_node(fwnode), index); + + return NULL; +} +EXPORT_SYMBOL(fwnode_iomap); + /** * fwnode_graph_get_next_endpoint - Get next endpoint firmware node * @fwnode: Pointer to the parent firmware node diff --git a/include/linux/property.h b/include/linux/property.h index 88fa726a76df..6670d5a1ec2a 100644 --- a/include/linux/property.h +++ b/include/linux/property.h @@ -122,6 +122,8 @@ void fwnode_handle_put(struct fwnode_handle *fwnode); int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index); +void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int index); + unsigned int device_get_child_node_count(struct device *dev); static inline bool device_property_read_bool(struct device *dev,