[PATCH 0/4] drivers/misc: add rawio framework and drivers

[PATCH 0/4] drivers/misc: add rawio framework and drivers

[Bin Gao]

To read/write registers from a device is very important on embedded system,
especially SoC systems. Physically there could be different types of devices
based on bus tyes, e.g. PCI devices, I2C (slave)devices, I/O devices(memory
mapped), inter-processor devices, etc. Typically there are userland
tools from
PC Linux to access device registers, but on some embedded system initrd and
rootfs come with a minimal busybox and most useful userland tools are not
available. To add these tools back to rootfs is not convenient either.
What's more, on some systems with runtime pm enabled, reading/writing
registers
from a device which is in low power state will cause problems. For these
reasons, to have some tools/interfaces directly from kernel space via debug
fs seems to be easy, cheap and convenient.

These patchsets are designed to  achieve above goals to ease
device driver and kernel debugging on embedded systems.

Rawio provides a framework to read/write registers from a bus, including
pci, i2c, I/O device(memory mapped), etc. based on debug fs.
Rawio bus drivers implement the read/write operation on a specific bus
on top of the rawio framework driver.
Currently only three bus drivers are available: pci, iomem and i2c.
But it's extremely easy to add more drivers on top of the framework
if needed.

 drivers/misc/Kconfig             |   1 +
 drivers/misc/Makefile            |   1 +
 drivers/misc/rawio/Kconfig       |  59 +++++
 drivers/misc/rawio/Makefile      |   4 +
 drivers/misc/rawio/rawio.c       | 514
+++++++++++++++++++++++++++++++++++++++
 drivers/misc/rawio/rawio_i2c.c   | 224 +++++++++++++++++
 drivers/misc/rawio/rawio_iomem.c | 401 ++++++++++++++++++++++++++++++
 drivers/misc/rawio/rawio_pci.c   | 235 ++++++++++++++++++
 include/linux/rawio.h            |  78 ++++++
 9 files changed, 1517 insertions(+)
 create mode 100644 drivers/misc/rawio/Kconfig
 create mode 100644 drivers/misc/rawio/Makefile
 create mode 100644 drivers/misc/rawio/rawio.c
 create mode 100644 drivers/misc/rawio/rawio_i2c.c
 create mode 100644 drivers/misc/rawio/rawio_iomem.c
 create mode 100644 drivers/misc/rawio/rawio_pci.c
 create mode 100644 include/linux/rawio.h

Re: [PATCH 0/4] drivers/misc: add rawio framework and drivers

[Greg Kroah-Hartman]

On Mon, Oct 21, 2013 at 05:03:17PM -0700, Bin Gao wrote:
> To read/write registers from a device is very important on embedded system,
> especially SoC systems. Physically there could be different types of devices
> based on bus tyes, e.g. PCI devices, I2C (slave)devices, I/O devices(memory
> mapped), inter-processor devices, etc. Typically there are userland
> tools from
> PC Linux to access device registers, but on some embedded system initrd and
> rootfs come with a minimal busybox and most useful userland tools are not
> available. To add these tools back to rootfs is not convenient either.
> What's more, on some systems with runtime pm enabled, reading/writing
> registers
> from a device which is in low power state will cause problems. For these
> reasons, to have some tools/interfaces directly from kernel space via debug
> fs seems to be easy, cheap and convenient.

So, just because userspace is "hard" you want to add stuff to the kernel
instead.

Sorry, but for over the past decade, we have been doing just the
opposite, if things can be done in userspace, then they should be done
there.  So for us to go in the opposite direction, like these patches
show, would be a major change.

> These patchsets are designed to  achieve above goals to ease
> device driver and kernel debugging on embedded systems.
> 
> Rawio provides a framework to read/write registers from a bus, including
> pci, i2c, I/O device(memory mapped), etc. based on debug fs.
> Rawio bus drivers implement the read/write operation on a specific bus
> on top of the rawio framework driver.
> Currently only three bus drivers are available: pci, iomem and i2c.

You can already do this today for PCI with the UIO framework, right?
Why duplicate that functionality here with another userapce API that we
will then have to maintain for the next 40+ years?

> But it's extremely easy to add more drivers on top of the framework
> if needed.
> 
>  drivers/misc/Kconfig             |   1 +
>  drivers/misc/Makefile            |   1 +
>  drivers/misc/rawio/Kconfig       |  59 +++++
>  drivers/misc/rawio/Makefile      |   4 +
>  drivers/misc/rawio/rawio.c       | 514
> +++++++++++++++++++++++++++++++++++++++

All of your patches are line-wrapped and totally fail to apply, so even
if we wanted to take this type of changes, I couldn't :(

Have you run these proposed changes by any of the Intel kernel
developers?  What did they say to them?

If not, why haven't you, isn't that a resource you should be using for
things like this?

greg k-h

Re: [PATCH 0/4] drivers/misc: add rawio framework and drivers

[Guenter Roeck]

On Tue, Oct 22, 2013 at 06:44:06AM +0100, Greg Kroah-Hartman wrote:
> On Mon, Oct 21, 2013 at 05:03:17PM -0700, Bin Gao wrote:
> > To read/write registers from a device is very important on embedded system,
> > especially SoC systems. Physically there could be different types of devices
> > based on bus tyes, e.g. PCI devices, I2C (slave)devices, I/O devices(memory
> > mapped), inter-processor devices, etc. Typically there are userland
> > tools from
> > PC Linux to access device registers, but on some embedded system initrd and
> > rootfs come with a minimal busybox and most useful userland tools are not
> > available. To add these tools back to rootfs is not convenient either.
> > What's more, on some systems with runtime pm enabled, reading/writing
> > registers
> > from a device which is in low power state will cause problems. For these
> > reasons, to have some tools/interfaces directly from kernel space via debug
> > fs seems to be easy, cheap and convenient.
> 
> So, just because userspace is "hard" you want to add stuff to the kernel
> instead.
> 
> Sorry, but for over the past decade, we have been doing just the
> opposite, if things can be done in userspace, then they should be done
> there.  So for us to go in the opposite direction, like these patches
> show, would be a major change.
> 
> > These patchsets are designed to  achieve above goals to ease
> > device driver and kernel debugging on embedded systems.
> > 
> > Rawio provides a framework to read/write registers from a bus, including
> > pci, i2c, I/O device(memory mapped), etc. based on debug fs.
> > Rawio bus drivers implement the read/write operation on a specific bus
> > on top of the rawio framework driver.
> > Currently only three bus drivers are available: pci, iomem and i2c.
> 
> You can already do this today for PCI with the UIO framework, right?
> Why duplicate that functionality here with another userapce API that we
> will then have to maintain for the next 40+ years?
> 
Same for i2c, where the same functionality is supported through i2c-tools and
the i2c-dev driver. Adding i2c-tools to initramfs and/or to the root file system
should not be that much of an issue, much less than having to maintain two APIs
for the same purpose.

Guenter

Re: [PATCH 0/4] drivers/misc: add rawio framework and drivers

[Bin Gao]

On Tue, Oct 22, 2013 at 10:14:00AM -0700, Guenter Roeck wrote:
> > 
> > You can already do this today for PCI with the UIO framework, right?
> > Why duplicate that functionality here with another userapce API that we
> > will then have to maintain for the next 40+ years?
> > 
> Same for i2c, where the same functionality is supported through i2c-tools and
> the i2c-dev driver. Adding i2c-tools to initramfs and/or to the root file system
> should not be that much of an issue, much less than having to maintain two APIs
> for the same purpose.
> 
> Guenter

For PCI and memory mapped I/O devices, we have the runtime pm issue that has to
be addressed from kernel space.
For I2C slave devices, there is no such a issue so yes i2c-tools and i2c-dev drivers
are fine. But the two APIs are required by PCI and memory mapped I/O devices anyway.

Re: [PATCH 0/4] drivers/misc: add rawio framework and drivers

[Greg Kroah-Hartman]

On Tue, Oct 22, 2013 at 11:50:04AM -0700, Bin Gao wrote:
> On Tue, Oct 22, 2013 at 10:14:00AM -0700, Guenter Roeck wrote:
> > > 
> > > You can already do this today for PCI with the UIO framework, right?
> > > Why duplicate that functionality here with another userapce API that we
> > > will then have to maintain for the next 40+ years?
> > > 
> > Same for i2c, where the same functionality is supported through i2c-tools and
> > the i2c-dev driver. Adding i2c-tools to initramfs and/or to the root file system
> > should not be that much of an issue, much less than having to maintain two APIs
> > for the same purpose.
> > 
> > Guenter
> 
> For PCI and memory mapped I/O devices, we have the runtime pm issue that has to
> be addressed from kernel space.

Then create a UIO pci driver and you should be fine, right?  That's what
others do today and it works quite well for them.

greg k-h

Re: [PATCH 0/4] drivers/misc: add rawio framework and drivers

[Bin Gao]

On Tue, Oct 22, 2013 at 06:44:06AM +0100, Greg Kroah-Hartman wrote:
> So, just because userspace is "hard" you want to add stuff to the kernel
> instead.
> 
Well, there are other reasons - "hard" is just one of them.
For instance, on some platforms with runtime pm enabled, access to registers
of a device which is in low power state will cause problems(syste reboot, etc.).
You can only wake it up to running state by runtime API from kernel space.

> Sorry, but for over the past decade, we have been doing just the
> opposite, if things can be done in userspace, then they should be done
> there.  So for us to go in the opposite direction, like these patches
> show, would be a major change.
> 
Agree, but as mentioned above, for some situation we can't do it from
user space.

> You can already do this today for PCI with the UIO framework, right?
> Why duplicate that functionality here with another userapce API that we
> will then have to maintain for the next 40+ years?
> 
No, UIO is not appropriate for my requirement.
The thing I need is to dump any registers just by 2 simple commands.

> All of your patches are line-wrapped and totally fail to apply, so even
> if we wanted to take this type of changes, I couldn't :(
> 
Sorry for that. I recently upgraded my email client, will fix it next posting.

> Have you run these proposed changes by any of the Intel kernel
> developers?  What did they say to them?
> 
> If not, why haven't you, isn't that a resource you should be using for
> things like this?
> 
Why you had these strange questions?
Over years, we have been maintaining and using these drivers internally 
for various purpose across our group for SoC pre-silicon and post-silicon
degugging, e.g. IOAPIC RTE dumping, GPIO tunning, raw device degugging
without a driver(i2c, spi, uart), etc., etc., ...
Trying to push some existed codes upstream is not a bad thing.

> greg k-h

Re: [PATCH 0/4] drivers/misc: add rawio framework and drivers

[Greg Kroah-Hartman]

On Tue, Oct 22, 2013 at 11:19:30AM -0700, Bin Gao wrote:
> On Tue, Oct 22, 2013 at 06:44:06AM +0100, Greg Kroah-Hartman wrote:
> > So, just because userspace is "hard" you want to add stuff to the kernel
> > instead.
> > 
> Well, there are other reasons - "hard" is just one of them.
> For instance, on some platforms with runtime pm enabled, access to registers
> of a device which is in low power state will cause problems(syste reboot, etc.).
> You can only wake it up to running state by runtime API from kernel space.

Then write a UIO pci driver, that should work, right?

> > Sorry, but for over the past decade, we have been doing just the
> > opposite, if things can be done in userspace, then they should be done
> > there.  So for us to go in the opposite direction, like these patches
> > show, would be a major change.
> > 
> Agree, but as mentioned above, for some situation we can't do it from
> user space.
> 
> > You can already do this today for PCI with the UIO framework, right?
> > Why duplicate that functionality here with another userapce API that we
> > will then have to maintain for the next 40+ years?
> > 
> No, UIO is not appropriate for my requirement.

I don't know what your "requirements" are.

> The thing I need is to dump any registers just by 2 simple commands.

I find it hard to believe that your "requirement" dictates the exact
method of _how_ you get access to this hardware.

> > Have you run these proposed changes by any of the Intel kernel
> > developers?  What did they say to them?
> > 
> > If not, why haven't you, isn't that a resource you should be using for
> > things like this?
> > 
> Why you had these strange questions?

It's something I ask any Intel developer who sends odd patches that have
obviously not been reviewed by Intel's kernel developers.  They are a
resource you should be using to tell you these types of things, don't
make me, a community member, tell you this.

> Over years, we have been maintaining and using these drivers internally 
> for various purpose across our group for SoC pre-silicon and post-silicon
> degugging, e.g. IOAPIC RTE dumping, GPIO tunning, raw device degugging
> without a driver(i2c, spi, uart), etc., etc., ...
> Trying to push some existed codes upstream is not a bad thing.

Trying to push code that is incorrect is a bad thing.  Just because it
has been living in a private tree for X number of years is not an
excuse, because you are now asking me and others to maintain this API
you have created (which is undocumented) for the next 40+ years.

Again, use the interfaces we already have, and if they are not
sufficient for whatever reason, help make them better.  Don't create
whole new ones just because someone 5+ years ago didn't know about them,
or that they weren't even present at that time.  That's not the
community's fault, it's yours.

greg k-h