linux-kernel - Re: [PATCH] Input: ambakmi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 13 Jul 2014 19:21:23 +0100
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Julia Lawall <julia.lawall@...6.fr>
Cc:	Himangi Saraogi <himangi774@...il.com>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Input: ambakmi - Use managed interfaces

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@...00000/usb@...84000
  02184000-021841ff : /soc/aips-bus@...00000/usb@...84000
02184200-021843ff : /soc/aips-bus@...00000/usb@...84200
  02184200-021843ff : /soc/aips-bus@...00000/usb@...84200

which is really pointless duplicating the resource name like that.  It
conveys no additional useful information.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/