| Message ID | 1401989181-4712-2-git-send-email-matthias.bgg@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jun 5, 2014 at 12:26 PM, Matthias Brugger <matthias.bgg@gmail.com> wrote: > A call to of_iomap does not request the memory region. > This patch adds the function of_io_request_and_map which requests > the memory region before mapping it. > > Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com> > --- > drivers/of/address.c | 28 ++++++++++++++++++++++++++++ > include/linux/of_address.h | 8 ++++++++ > 2 files changed, 36 insertions(+) > > diff --git a/drivers/of/address.c b/drivers/of/address.c > index cb4242a..c55b107 100644 > --- a/drivers/of/address.c > +++ b/drivers/of/address.c > @@ -721,3 +721,31 @@ void __iomem *of_iomap(struct device_node *np, int index) > return ioremap(res.start, resource_size(&res)); > } > EXPORT_SYMBOL(of_iomap); > + > +/** > + * of_io_request_and_map - Requests a resource and maps the memory mapped IO > + * for a given device_node I believe docbook requires this to be 1 line. > + * @device: the device whose io range will be mapped > + * @index: index of the io range > + * @name: name of the resource > + * > + * Returns a pointer to the requested and mapped memory > + */ > +void __iomem *of_io_request_and_map(struct device_node *np, int index, char *name) > +{ > + struct resource res; > + void __iomem *mem; > + > + if (of_address_to_resource(np, index, &res)) > + return NULL; > + > + if (!request_mem_region(res.start, resource_size(&res), name)) Use the np->name here and drop the name parameter. > + return NULL; > + > + mem = ioremap(res.start, resource_size(&res)); > + if (!mem) > + release_mem_region(res.start, resource_size(&res)); > + > + return mem; > +} > +EXPORT_SYMBOL(of_io_request_and_map); > diff --git a/include/linux/of_address.h b/include/linux/of_address.h > index 5f6ed6b..725b875 100644 > --- a/include/linux/of_address.h > +++ b/include/linux/of_address.h > @@ -96,6 +96,8 @@ static inline struct of_pci_range *of_pci_range_parser_one( > extern int of_address_to_resource(struct device_node *dev, int index, > struct resource *r); > void __iomem *of_iomap(struct device_node *node, int index); > +void __iomem *of_io_request_and_map(struct device_node *device, > + int index, char *name); > #else > static inline int of_address_to_resource(struct device_node *dev, int index, > struct resource *r) > @@ -107,6 +109,12 @@ static inline void __iomem *of_iomap(struct device_node *device, int index) > { > return NULL; > } > + > +static inline void __iomem *of_io_request_and_map(struct device_node *device, > + int index, char *name) > +{ > + return NULL; > +} > #endif > > #if defined(CONFIG_OF_ADDRESS) && defined(CONFIG_PCI) > -- > 1.7.9.5 >
On 06/06/2014 08:19 AM, Rob Herring wrote: > On Thu, Jun 5, 2014 at 12:26 PM, Matthias Brugger > <matthias.bgg@gmail.com> wrote: >> A call to of_iomap does not request the memory region. >> This patch adds the function of_io_request_and_map which requests >> the memory region before mapping it. >> >> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com> >> --- >> drivers/of/address.c | 28 ++++++++++++++++++++++++++++ >> include/linux/of_address.h | 8 ++++++++ >> 2 files changed, 36 insertions(+) >> >> diff --git a/drivers/of/address.c b/drivers/of/address.c >> index cb4242a..c55b107 100644 >> --- a/drivers/of/address.c >> +++ b/drivers/of/address.c >> @@ -721,3 +721,31 @@ void __iomem *of_iomap(struct device_node *np, int index) >> return ioremap(res.start, resource_size(&res)); >> } >> EXPORT_SYMBOL(of_iomap); >> + >> +/** >> + * of_io_request_and_map - Requests a resource and maps the memory mapped IO >> + * for a given device_node > > I believe docbook requires this to be 1 line. No longer so. That was fixed a couple of years ago.
On Fri, 6 Jun 2014 10:19:28 -0500, Rob Herring <robherring2@gmail.com> wrote: > On Thu, Jun 5, 2014 at 12:26 PM, Matthias Brugger > <matthias.bgg@gmail.com> wrote: > > A call to of_iomap does not request the memory region. > > This patch adds the function of_io_request_and_map which requests > > the memory region before mapping it. > > > > Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com> > > --- > > drivers/of/address.c | 28 ++++++++++++++++++++++++++++ > > include/linux/of_address.h | 8 ++++++++ > > 2 files changed, 36 insertions(+) > > > > diff --git a/drivers/of/address.c b/drivers/of/address.c > > index cb4242a..c55b107 100644 > > --- a/drivers/of/address.c > > +++ b/drivers/of/address.c > > @@ -721,3 +721,31 @@ void __iomem *of_iomap(struct device_node *np, int index) > > return ioremap(res.start, resource_size(&res)); > > } > > EXPORT_SYMBOL(of_iomap); > > + > > +/** > > + * of_io_request_and_map - Requests a resource and maps the memory mapped IO > > + * for a given device_node > > I believe docbook requires this to be 1 line. > > > + * @device: the device whose io range will be mapped > > + * @index: index of the io range > > + * @name: name of the resource > > + * > > + * Returns a pointer to the requested and mapped memory > > + */ > > +void __iomem *of_io_request_and_map(struct device_node *np, int index, char *name) > > +{ > > + struct resource res; > > + void __iomem *mem; > > + > > + if (of_address_to_resource(np, index, &res)) > > + return NULL; > > + > > + if (!request_mem_region(res.start, resource_size(&res), name)) > > Use the np->name here and drop the name parameter. Name here would be the name of the owner (the driver), not the name of the node. Passing the name separately is fine by me. Acked-by: Grant Likely <grant.likely@linaro.org> You can merge this with the rest of the series. g. > > > + return NULL; > > + > > + mem = ioremap(res.start, resource_size(&res)); > > + if (!mem) > > + release_mem_region(res.start, resource_size(&res)); > > + > > + return mem; > > +} > > +EXPORT_SYMBOL(of_io_request_and_map); > > diff --git a/include/linux/of_address.h b/include/linux/of_address.h > > index 5f6ed6b..725b875 100644 > > --- a/include/linux/of_address.h > > +++ b/include/linux/of_address.h > > @@ -96,6 +96,8 @@ static inline struct of_pci_range *of_pci_range_parser_one( > > extern int of_address_to_resource(struct device_node *dev, int index, > > struct resource *r); > > void __iomem *of_iomap(struct device_node *node, int index); > > +void __iomem *of_io_request_and_map(struct device_node *device, > > + int index, char *name); > > #else > > static inline int of_address_to_resource(struct device_node *dev, int index, > > struct resource *r) > > @@ -107,6 +109,12 @@ static inline void __iomem *of_iomap(struct device_node *device, int index) > > { > > return NULL; > > } > > + > > +static inline void __iomem *of_io_request_and_map(struct device_node *device, > > + int index, char *name) > > +{ > > + return NULL; > > +} > > #endif > > > > #if defined(CONFIG_OF_ADDRESS) && defined(CONFIG_PCI) > > -- > > 1.7.9.5 > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Sat, Jun 7, 2014 at 12:59 AM, Grant Likely <grant.likely@secretlab.ca> wrote: > On Fri, 6 Jun 2014 10:19:28 -0500, Rob Herring <robherring2@gmail.com> wrote: >> On Thu, Jun 5, 2014 at 12:26 PM, Matthias Brugger >> <matthias.bgg@gmail.com> wrote: >> > A call to of_iomap does not request the memory region. >> > This patch adds the function of_io_request_and_map which requests >> > the memory region before mapping it. >> > >> > Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com> >> > --- >> > drivers/of/address.c | 28 ++++++++++++++++++++++++++++ >> > include/linux/of_address.h | 8 ++++++++ >> > 2 files changed, 36 insertions(+) >> > >> > diff --git a/drivers/of/address.c b/drivers/of/address.c >> > index cb4242a..c55b107 100644 >> > --- a/drivers/of/address.c >> > +++ b/drivers/of/address.c >> > @@ -721,3 +721,31 @@ void __iomem *of_iomap(struct device_node *np, int index) >> > return ioremap(res.start, resource_size(&res)); >> > } >> > EXPORT_SYMBOL(of_iomap); >> > + >> > +/** >> > + * of_io_request_and_map - Requests a resource and maps the memory mapped IO >> > + * for a given device_node >> >> I believe docbook requires this to be 1 line. >> >> > + * @device: the device whose io range will be mapped >> > + * @index: index of the io range >> > + * @name: name of the resource >> > + * >> > + * Returns a pointer to the requested and mapped memory >> > + */ >> > +void __iomem *of_io_request_and_map(struct device_node *np, int index, char *name) >> > +{ >> > + struct resource res; >> > + void __iomem *mem; >> > + >> > + if (of_address_to_resource(np, index, &res)) >> > + return NULL; >> > + >> > + if (!request_mem_region(res.start, resource_size(&res), name)) >> >> Use the np->name here and drop the name parameter. > > Name here would be the name of the owner (the driver), not the name of > the node. Passing the name separately is fine by me. This function is for when there is no driver. If there is a driver, the devm_* functions should be used. I would like to see some standard naming and consistency here rather than allowing random strings or NULL to be passed by the caller. Rob
2014-06-09 16:36 GMT+02:00 Rob Herring <robherring2@gmail.com>: > On Sat, Jun 7, 2014 at 12:59 AM, Grant Likely <grant.likely@secretlab.ca> wrote: >> On Fri, 6 Jun 2014 10:19:28 -0500, Rob Herring <robherring2@gmail.com> wrote: >>> On Thu, Jun 5, 2014 at 12:26 PM, Matthias Brugger >>> <matthias.bgg@gmail.com> wrote: >>> > A call to of_iomap does not request the memory region. >>> > This patch adds the function of_io_request_and_map which requests >>> > the memory region before mapping it. >>> > >>> > Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com> >>> > --- >>> > drivers/of/address.c | 28 ++++++++++++++++++++++++++++ >>> > include/linux/of_address.h | 8 ++++++++ >>> > 2 files changed, 36 insertions(+) >>> > >>> > diff --git a/drivers/of/address.c b/drivers/of/address.c >>> > index cb4242a..c55b107 100644 >>> > --- a/drivers/of/address.c >>> > +++ b/drivers/of/address.c >>> > @@ -721,3 +721,31 @@ void __iomem *of_iomap(struct device_node *np, int index) >>> > return ioremap(res.start, resource_size(&res)); >>> > } >>> > EXPORT_SYMBOL(of_iomap); >>> > + >>> > +/** >>> > + * of_io_request_and_map - Requests a resource and maps the memory mapped IO >>> > + * for a given device_node >>> >>> I believe docbook requires this to be 1 line. >>> >>> > + * @device: the device whose io range will be mapped >>> > + * @index: index of the io range >>> > + * @name: name of the resource >>> > + * >>> > + * Returns a pointer to the requested and mapped memory >>> > + */ >>> > +void __iomem *of_io_request_and_map(struct device_node *np, int index, char *name) >>> > +{ >>> > + struct resource res; >>> > + void __iomem *mem; >>> > + >>> > + if (of_address_to_resource(np, index, &res)) >>> > + return NULL; >>> > + >>> > + if (!request_mem_region(res.start, resource_size(&res), name)) >>> >>> Use the np->name here and drop the name parameter. >> >> Name here would be the name of the owner (the driver), not the name of >> the node. Passing the name separately is fine by me. > > This function is for when there is no driver. If there is a driver, > the devm_* functions should be used. I would like to see some standard > naming and consistency here rather than allowing random strings or > NULL to be passed by the caller. A driver can request and map more then one memory region, so it is convenient that they have different names. Therefor the extra parameter. Another possibility would be to use np->name together with the index to get unique names for every region requested by the driver Regards, Matthias
On Mon, Jun 9, 2014 at 3:36 PM, Rob Herring <robherring2@gmail.com> wrote: > On Sat, Jun 7, 2014 at 12:59 AM, Grant Likely <grant.likely@secretlab.ca> wrote: >> On Fri, 6 Jun 2014 10:19:28 -0500, Rob Herring <robherring2@gmail.com> wrote: >>> On Thu, Jun 5, 2014 at 12:26 PM, Matthias Brugger >>> <matthias.bgg@gmail.com> wrote: >>> > A call to of_iomap does not request the memory region. >>> > This patch adds the function of_io_request_and_map which requests >>> > the memory region before mapping it. >>> > >>> > Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com> >>> > --- >>> > drivers/of/address.c | 28 ++++++++++++++++++++++++++++ >>> > include/linux/of_address.h | 8 ++++++++ >>> > 2 files changed, 36 insertions(+) >>> > >>> > diff --git a/drivers/of/address.c b/drivers/of/address.c >>> > index cb4242a..c55b107 100644 >>> > --- a/drivers/of/address.c >>> > +++ b/drivers/of/address.c >>> > @@ -721,3 +721,31 @@ void __iomem *of_iomap(struct device_node *np, int index) >>> > return ioremap(res.start, resource_size(&res)); >>> > } >>> > EXPORT_SYMBOL(of_iomap); >>> > + >>> > +/** >>> > + * of_io_request_and_map - Requests a resource and maps the memory mapped IO >>> > + * for a given device_node >>> >>> I believe docbook requires this to be 1 line. >>> >>> > + * @device: the device whose io range will be mapped >>> > + * @index: index of the io range >>> > + * @name: name of the resource >>> > + * >>> > + * Returns a pointer to the requested and mapped memory >>> > + */ >>> > +void __iomem *of_io_request_and_map(struct device_node *np, int index, char *name) >>> > +{ >>> > + struct resource res; >>> > + void __iomem *mem; >>> > + >>> > + if (of_address_to_resource(np, index, &res)) >>> > + return NULL; >>> > + >>> > + if (!request_mem_region(res.start, resource_size(&res), name)) >>> >>> Use the np->name here and drop the name parameter. >> >> Name here would be the name of the owner (the driver), not the name of >> the node. Passing the name separately is fine by me. > > This function is for when there is no driver. If there is a driver, > the devm_* functions should be used. I would like to see some standard > naming and consistency here rather than allowing random strings or > NULL to be passed by the caller. The node name doesn't provide a whole lot of help here though because we want to identify the code creating the resource, not the node. I'm not going to make a big deal about it. I've told Matthias on IRC that he can go ahead and merge it because it is a very minor issue. If you feel strongly about it, then we'll get him to change it or do a follow-up patch to change the function signature. g.
diff --git a/drivers/of/address.c b/drivers/of/address.c index cb4242a..c55b107 100644 --- a/drivers/of/address.c +++ b/drivers/of/address.c @@ -721,3 +721,31 @@ void __iomem *of_iomap(struct device_node *np, int index) return ioremap(res.start, resource_size(&res)); } EXPORT_SYMBOL(of_iomap); + +/** + * of_io_request_and_map - Requests a resource and maps the memory mapped IO + * for a given device_node + * @device: the device whose io range will be mapped + * @index: index of the io range + * @name: name of the resource + * + * Returns a pointer to the requested and mapped memory + */ +void __iomem *of_io_request_and_map(struct device_node *np, int index, char *name) +{ + struct resource res; + void __iomem *mem; + + if (of_address_to_resource(np, index, &res)) + return NULL; + + if (!request_mem_region(res.start, resource_size(&res), name)) + return NULL; + + mem = ioremap(res.start, resource_size(&res)); + if (!mem) + release_mem_region(res.start, resource_size(&res)); + + return mem; +} +EXPORT_SYMBOL(of_io_request_and_map); diff --git a/include/linux/of_address.h b/include/linux/of_address.h index 5f6ed6b..725b875 100644 --- a/include/linux/of_address.h +++ b/include/linux/of_address.h @@ -96,6 +96,8 @@ static inline struct of_pci_range *of_pci_range_parser_one( extern int of_address_to_resource(struct device_node *dev, int index, struct resource *r); void __iomem *of_iomap(struct device_node *node, int index); +void __iomem *of_io_request_and_map(struct device_node *device, + int index, char *name); #else static inline int of_address_to_resource(struct device_node *dev, int index, struct resource *r) @@ -107,6 +109,12 @@ static inline void __iomem *of_iomap(struct device_node *device, int index) { return NULL; } + +static inline void __iomem *of_io_request_and_map(struct device_node *device, + int index, char *name) +{ + return NULL; +} #endif #if defined(CONFIG_OF_ADDRESS) && defined(CONFIG_PCI)
A call to of_iomap does not request the memory region. This patch adds the function of_io_request_and_map which requests the memory region before mapping it. Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com> --- drivers/of/address.c | 28 ++++++++++++++++++++++++++++ include/linux/of_address.h | 8 ++++++++ 2 files changed, 36 insertions(+)