Re: [PATCH] Input: i8042-io - Exclude mips platforms when allocating/deallocating IO regions.

[Ralf Baechle]

On Tue, Jan 28, 2014 at 06:25:39AM +0000, Raghu Gandham wrote:
> Date:   Tue, 28 Jan 2014 06:25:39 +0000
> From: Raghu Gandham <Raghu.Gandham@xxxxxxxxxx>
> To: Aaro Koskinen <aaro.koskinen@xxxxxx>, Dmitry Torokhov
>  <dmitry.torokhov@xxxxxxxxx>
> CC: "linux-input@xxxxxxxxxxxxxxx" <linux-input@xxxxxxxxxxxxxxx>,
>  "linux-mips@xxxxxxxxxxxxxx" <linux-mips@xxxxxxxxxxxxxx>,
>  "linux-kernel@xxxxxxxxxxxxxxx" <linux-kernel@xxxxxxxxxxxxxxx>
> Subject: RE: [PATCH] Input: i8042-io - Exclude mips platforms when
>  allocating/deallocating IO regions.
> Content-Type: text/plain; charset="us-ascii"
> 
> Hi Aaro,
> 
> > 
> > On Sun, Jan 26, 2014 at 10:56:38PM -0800, Dmitry Torokhov wrote:
> > > On Mon, Jan 27, 2014 at 12:32:36AM +0000, Raghu Gandham wrote:
> > > > > On Sat, Jan 25, 2014 at 11:01:54AM -0800, Raghu Gandham wrote:
> > > > > > The standard IO regions are already reserved by the platform
> > > > > > code on most MIPS devices(malta, cobalt, sni). The Commit
> > > > > > 197a1e96c8be5b6005145af3a4c0e45e2d651444
> > > > > > ("Input: i8042-io - fix up region handling on MIPS") introduced
> > > > > > a bug on these MIPS platforms causing i8042 driver to fail when
> > > > > > trying to reserve IO ports.
> > > > > > Prior to the above mentioned commit request_region is skipped on
> > > > > > MIPS but release_region is called.
> > > > > >
> > > > > > This patch reverts commit
> > > > > > 197a1e96c8be5b6005145af3a4c0e45e2d651444
> > > > > > and also avoids calling release_region for MIPS.
> > > > >
> > > > > The problem is that IO regions are reserved on _most_, but not
> > > > > _all_ devices.
> > > > > MIPS should figure out what they want to do with i8042 registers
> > > > > and be consistent on all devices.
> > > >
> > > > Please examine the attached patch which went upstream in April of 2004.
> > > > Since then it had been a convention not to call request_region
> > > > routine in
> > > > i8042 for MIPS. The attached patch had a glitch that it guarded
> > > > request_region in i8042-io.h but skipped guarding release_region in
> > > > i8042-io.h. I believe that the issue Aaro saw was due to this
> > > > glitch. Below is the error quoted in Aaro's commit message.
> > > >
> > > >     [    2.112000] Trying to free nonexistent resource <0000000000000060-
> > 000000000000006f>
> > > >
> > > > My patch reinstates the convention followed on MIPS devices along
> > > > with fixing Aaro's issue.
> > >
> > > I assume that Aaro did test his patch and on his box request_region()
> > > succeeds. That would indicate that various MIPS sub-arches still not
> > > settled on the topic.
> > 
> > request_region() succeeds on Loongson and OCTEON.
> 
> This would mean that before your patch in oct of 2012, Loongson and Octeon 
> were not reserving the IO space for i8042.
> 
> > 
> > On OCTEONs without PCI, request_region() will fail which is correct as there
> > is no I/O space.
> > 
> > I wasn't aware of that 2004 patch (it pre-dates GIT history of mainline Linux).
> > Why the regions are already reserved by the platform code?
> 
> The only information I have is the comment before request_region in i8042-io.h that
> touching data register on some platforms is flaky.  If your patch was primarily aimed at
> addressing the error message from release_region, the current patch I uploaded should
> also take care of it too. 

I think the patch (http://patchwork.linux-mips.org/patch/6419/) should be
applied.  The argumentation for reserving ports in the platform code are
the same as on x86 - touch the registers and bad things may happen.  This
is because a fair number of older MIPS platforms were based on x86 chipsets
or at least are using very similar designs.

The fact that on certain platforms such as Loongson, some Octeon systems
and others the request_region() call to reserve the keyboard call succeeds
is by accident not design.

I wish i8042.c was a real platform driver, not using platform_create_bundle.
That would leave a natural place in the arch/platform code to deal with
I/O port allocation and platform_device creation, as necessary for a
platform.

  Ralf
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/