Re: [PATCH] Input: ambakmi - Use managed interfaces

[Julia Lawall <julia.lawall@lip6.fr>]

On Sun, 13 Jul 2014, Russell King - ARM Linux wrote:

> On Sun, Jul 13, 2014 at 08:11:29PM +0200, Julia Lawall wrote:
> > On Sun, 13 Jul 2014, Russell King - ARM Linux wrote:
> > 
> > > On Sun, Jul 13, 2014 at 11:00:51PM +0530, Himangi Saraogi wrote:
> > > > @@ -23,6 +23,7 @@
> > > >  #include <linux/clk.h>
> > > >  
> > > >  #include <asm/io.h>
> > > > +#include <linux/io.h>
> > > 
> > > NAK - please include either linux/io.h _or_ asm/io.h but not both.
> > > 
> > > > @@ -112,19 +113,10 @@ static int amba_kmi_probe(struct amba_device *dev,
> > > >  {
> > > >  	struct amba_kmi_port *kmi;
> > > >  	struct serio *io;
> > > > -	int ret;
> > > > -
> > > > -	ret = amba_request_regions(dev, NULL);
> > > > -	if (ret)
> > > > -		return ret;
> > > 
> > > I'm /really/ not happy about that going.
> > 
> > Could you explain why?  I looked at the code several times, and I couldn't 
> > see how it was different than request_mem_region, which is merged into 
> > devm_ioremap_resource.
> 
> Check what gets used for the name of the resource.
> 
> Now, consider that most devices when they are registered have their
> resource names set to the device name.
> 
> Then realise that devm_ioremap_resource() uses the resource name or
> the device name again.  So, what you end up with in /proc/iomem is
> a load of stupidity - you don't get to see there which drivers are
> making use of the resources, only a load of duplicated information
> about what devices are using the regions.
> 
> This is contary to other bus types (like PCI) where the _device_ takes
> the non-busy parent resource, and the driver takes the busy resource
> using the _driver_ name, not the device name.  The exception to this
> is network drivers which have in the past used the network device
> name.
> 
> Here's an example.  x86:
> 
>   fc000000-fc01ffff : 0000:00:19.0	<--- device name
>     fc000000-fc01ffff : e1000e		<--- driver name
>   fc020000-fc023fff : 0000:00:1b.0	<--- device name
>     fc020000-fc023fff : ICH HD audio	<--- driver name
>   fc024000-fc024fff : 0000:00:03.3
>   fc025000-fc025fff : 0000:00:19.0	<--- device name
>     fc025000-fc025fff : e1000e		<--- driver name
>   fc226000-fc2267ff : 0000:00:1f.2	<--- device name
>     fc226000-fc2267ff : ahci		<--- driver name
> 
> etc.  When using devm_ioremap_resource() this ends up as:
> 
> 02184000-021841ff : /soc/aips-bus@02100000/usb@02184000
>   02184000-021841ff : /soc/aips-bus@02100000/usb@02184000
> 02184200-021843ff : /soc/aips-bus@02100000/usb@02184200
>   02184200-021843ff : /soc/aips-bus@02100000/usb@02184200
> 
> which is really pointless duplicating the resource name like that.  It
> conveys no additional useful information.

OK, thanks very much.

julia