[v12,1/5] device property: Add fwnode_iomap()

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(+)

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
> > >    
>