Re: [PATCH v2] misc: Driver for Silicon Labs Si570 and compatibles

2011-04-22 Thread Guenter Roeck 
On Thu, 2011-04-21 at 19:34 -0400, Hans J. Koch wrote:
 On Thu, Apr 21, 2011 at 01:11:25PM +0200, Arnd Bergmann wrote:
  On Wednesday 20 April 2011, Guenter Roeck wrote:
   On Wed, Apr 20, 2011 at 12:44:30PM -0400, Arnd Bergmann wrote:
On Tuesday 19 April 2011, Guenter Roeck wrote:
 This driver adds support for Si570, Si571, Si598, and Si599
 programmable XO/VCXO.
 
 Signed-off-by: Guenter Roeck guenter.ro...@ericsson.com

This needs some more explanation of what the hardware is there for,
and why it's unlike everything else that we support in Linux.

   This is a generic configurable clock device. I'll be happy to add
   some text such as The device can be used for any application requiring
   a static or a dynamically configurable clock, such as serdes clocks.
   Not sure if that would add much value, though.
   
   Regarding unlike everything else, not sure if that is really correct.
   The DDS chips Jonathan mentioned do get pretty close, and there are
   other drivers providing support for clock chips, though typically more
   dedicated.  ics932s401 in misc is one example, and then there are all
   the tuner chips in media/common/tuners/.
  
  Isn't that what you'd normally call a 'struct clk' then?
  
If that is true, it should probably not have a user-visible
interface, but only an interface that can be used by other
kernel drivers.

   Depends. In our case, turns out the devices consuming the clock
   have user mode drivers. Lots of history there, but the chip vendors 
   provide those user mode drivers, and the teams responsible for
   integrating the drivers decided to not rewrite it to kernel mode drivers.
   Also, for special purposes such as margining, it is necessary to control
   the clock from userspace. So, for our use case, I need the user-visible
   interface. I _don't_ need the kernel interface, at least not right now,
   which is why I did not add it.
   
   Browsing through the web, it seems the chip is somewhat popular with
   Amateur Radio. No idea if it would ever be controlled for such a purpose
   from Linux, but if so, it would also require a user configurable 
   frequency.
   
   If there is a better place for such a driver than misc, please let me 
   know.
  
  When you say user mode driver, do you mean as in drivers/uio? (taking Hans
  on Cc for these)
 
 No UIO driver like that ever reached me.
 
  
  Those already have generic support for memory and interrupt resources,
  maybe we can just add a common way to associate a uio device with a struct 
  clk
  and provide a sysfs or ioctl interface to set a clock for a given device.
 
 I don't think the UIO framework is the right place for such a thing. If it's
 just this one driver that needs modification of a clock from userspace, then
 a sysfs attribute could be added to that driver. If there are several drivers
 that need this, then the clock framework should be extended.

Agreed. Also, the desire to control the clock frequency _through_ such a
driver (user mode or not) seems odd. Such an approach would be
inherently non-scalable (for my part I would have to support two drivers
already). Voltage regulators are not controlled through the drivers of
the chips they are providing power to either, but have an independent
existence.

Guenter


--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] misc: Driver for Silicon Labs Si570 and compatibles

2011-04-21 Thread Arnd Bergmann 
On Wednesday 20 April 2011, Guenter Roeck wrote:
 On Wed, Apr 20, 2011 at 12:44:30PM -0400, Arnd Bergmann wrote:
  On Tuesday 19 April 2011, Guenter Roeck wrote:
   This driver adds support for Si570, Si571, Si598, and Si599
   programmable XO/VCXO.
   
   Signed-off-by: Guenter Roeck guenter.ro...@ericsson.com
  
  This needs some more explanation of what the hardware is there for,
  and why it's unlike everything else that we support in Linux.
  
 This is a generic configurable clock device. I'll be happy to add
 some text such as The device can be used for any application requiring
 a static or a dynamically configurable clock, such as serdes clocks.
 Not sure if that would add much value, though.
 
 Regarding unlike everything else, not sure if that is really correct.
 The DDS chips Jonathan mentioned do get pretty close, and there are
 other drivers providing support for clock chips, though typically more
 dedicated.  ics932s401 in misc is one example, and then there are all
 the tuner chips in media/common/tuners/.

Isn't that what you'd normally call a 'struct clk' then?

  If that is true, it should probably not have a user-visible
  interface, but only an interface that can be used by other
  kernel drivers.
  
 Depends. In our case, turns out the devices consuming the clock
 have user mode drivers. Lots of history there, but the chip vendors 
 provide those user mode drivers, and the teams responsible for
 integrating the drivers decided to not rewrite it to kernel mode drivers.
 Also, for special purposes such as margining, it is necessary to control
 the clock from userspace. So, for our use case, I need the user-visible
 interface. I _don't_ need the kernel interface, at least not right now,
 which is why I did not add it.
 
 Browsing through the web, it seems the chip is somewhat popular with
 Amateur Radio. No idea if it would ever be controlled for such a purpose
 from Linux, but if so, it would also require a user configurable frequency.
 
 If there is a better place for such a driver than misc, please let me know.

When you say user mode driver, do you mean as in drivers/uio? (taking Hans
on Cc for these)

Those already have generic support for memory and interrupt resources,
maybe we can just add a common way to associate a uio device with a struct clk
and provide a sysfs or ioctl interface to set a clock for a given device.

Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] misc: Driver for Silicon Labs Si570 and compatibles

2011-04-21 Thread Guenter Roeck 
On Thu, Apr 21, 2011 at 07:11:25AM -0400, Arnd Bergmann wrote:
 On Wednesday 20 April 2011, Guenter Roeck wrote:
  On Wed, Apr 20, 2011 at 12:44:30PM -0400, Arnd Bergmann wrote:
   On Tuesday 19 April 2011, Guenter Roeck wrote:
This driver adds support for Si570, Si571, Si598, and Si599
programmable XO/VCXO.

Signed-off-by: Guenter Roeck guenter.ro...@ericsson.com
   
   This needs some more explanation of what the hardware is there for,
   and why it's unlike everything else that we support in Linux.
   
  This is a generic configurable clock device. I'll be happy to add
  some text such as The device can be used for any application requiring
  a static or a dynamically configurable clock, such as serdes clocks.
  Not sure if that would add much value, though.
  
  Regarding unlike everything else, not sure if that is really correct.
  The DDS chips Jonathan mentioned do get pretty close, and there are
  other drivers providing support for clock chips, though typically more
  dedicated.  ics932s401 in misc is one example, and then there are all
  the tuner chips in media/common/tuners/.
 
 Isn't that what you'd normally call a 'struct clk' then?
 
Yes, it would be nice to have that as part of the kernel and not architecture
specific.

   If that is true, it should probably not have a user-visible
   interface, but only an interface that can be used by other
   kernel drivers.
   
  Depends. In our case, turns out the devices consuming the clock
  have user mode drivers. Lots of history there, but the chip vendors 
  provide those user mode drivers, and the teams responsible for
  integrating the drivers decided to not rewrite it to kernel mode drivers.
  Also, for special purposes such as margining, it is necessary to control
  the clock from userspace. So, for our use case, I need the user-visible
  interface. I _don't_ need the kernel interface, at least not right now,
  which is why I did not add it.
  
  Browsing through the web, it seems the chip is somewhat popular with
  Amateur Radio. No idea if it would ever be controlled for such a purpose
  from Linux, but if so, it would also require a user configurable frequency.
  
  If there is a better place for such a driver than misc, please let me know.
 
 When you say user mode driver, do you mean as in drivers/uio? (taking Hans
 on Cc for these)
 
 Those already have generic support for memory and interrupt resources,
 maybe we can just add a common way to associate a uio device with a struct clk
 and provide a sysfs or ioctl interface to set a clock for a given device.
 
... and provide the clk infrastructure for x86, which is where I need it.

Guenter
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] misc: Driver for Silicon Labs Si570 and compatibles

2011-04-21 Thread Arnd Bergmann 
On Thursday 21 April 2011 17:56:58 Guenter Roeck wrote:
 On Thu, Apr 21, 2011 at 07:11:25AM -0400, Arnd Bergmann wrote:
  On Wednesday 20 April 2011, Guenter Roeck wrote:
  
  Isn't that what you'd normally call a 'struct clk' then?
  
 Yes, it would be nice to have that as part of the kernel and not architecture
 specific.

I think the generalization of the ARM code is currently being worked
on, so this should be there soon.

Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] misc: Driver for Silicon Labs Si570 and compatibles

2011-04-21 Thread Guenter Roeck 
On Thu, Apr 21, 2011 at 12:23:21PM -0400, Arnd Bergmann wrote:
 On Thursday 21 April 2011 17:56:58 Guenter Roeck wrote:
  On Thu, Apr 21, 2011 at 07:11:25AM -0400, Arnd Bergmann wrote:
   On Wednesday 20 April 2011, Guenter Roeck wrote:
   
   Isn't that what you'd normally call a 'struct clk' then?
   
  Yes, it would be nice to have that as part of the kernel and not 
  architecture
  specific.
 
 I think the generalization of the ARM code is currently being worked
 on, so this should be there soon.
 
Any pointers ?

Guenter
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] misc: Driver for Silicon Labs Si570 and compatibles

2011-04-21 Thread Arnd Bergmann 
On Thursday 21 April 2011 20:47:13 Guenter Roeck wrote:
 On Thu, Apr 21, 2011 at 12:23:21PM -0400, Arnd Bergmann wrote:
  
  I think the generalization of the ARM code is currently being worked
  on, so this should be there soon.
  
 Any pointers ?
 
I believe the most recent discussion is at
http://thread.gmane.org/gmane.linux.ports.arm.kernel/113631

Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] misc: Driver for Silicon Labs Si570 and compatibles

2011-04-21 Thread Guenter Roeck 
On Thu, 2011-04-21 at 15:00 -0400, Arnd Bergmann wrote:
 On Thursday 21 April 2011 20:47:13 Guenter Roeck wrote:
  On Thu, Apr 21, 2011 at 12:23:21PM -0400, Arnd Bergmann wrote:
   
   I think the generalization of the ARM code is currently being worked
   on, so this should be there soon.
   
  Any pointers ?
  
 I believe the most recent discussion is at
 http://thread.gmane.org/gmane.linux.ports.arm.kernel/113631
 
   Arnd

Looks like that is going to take a while. Oh well. I'll put this driver
on the backburner.

Guenter


--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] misc: Driver for Silicon Labs Si570 and compatibles

2011-04-21 Thread Hans J. Koch 
On Thu, Apr 21, 2011 at 01:11:25PM +0200, Arnd Bergmann wrote:
 On Wednesday 20 April 2011, Guenter Roeck wrote:
  On Wed, Apr 20, 2011 at 12:44:30PM -0400, Arnd Bergmann wrote:
   On Tuesday 19 April 2011, Guenter Roeck wrote:
This driver adds support for Si570, Si571, Si598, and Si599
programmable XO/VCXO.

Signed-off-by: Guenter Roeck guenter.ro...@ericsson.com
   
   This needs some more explanation of what the hardware is there for,
   and why it's unlike everything else that we support in Linux.
   
  This is a generic configurable clock device. I'll be happy to add
  some text such as The device can be used for any application requiring
  a static or a dynamically configurable clock, such as serdes clocks.
  Not sure if that would add much value, though.
  
  Regarding unlike everything else, not sure if that is really correct.
  The DDS chips Jonathan mentioned do get pretty close, and there are
  other drivers providing support for clock chips, though typically more
  dedicated.  ics932s401 in misc is one example, and then there are all
  the tuner chips in media/common/tuners/.
 
 Isn't that what you'd normally call a 'struct clk' then?
 
   If that is true, it should probably not have a user-visible
   interface, but only an interface that can be used by other
   kernel drivers.
   
  Depends. In our case, turns out the devices consuming the clock
  have user mode drivers. Lots of history there, but the chip vendors 
  provide those user mode drivers, and the teams responsible for
  integrating the drivers decided to not rewrite it to kernel mode drivers.
  Also, for special purposes such as margining, it is necessary to control
  the clock from userspace. So, for our use case, I need the user-visible
  interface. I _don't_ need the kernel interface, at least not right now,
  which is why I did not add it.
  
  Browsing through the web, it seems the chip is somewhat popular with
  Amateur Radio. No idea if it would ever be controlled for such a purpose
  from Linux, but if so, it would also require a user configurable frequency.
  
  If there is a better place for such a driver than misc, please let me know.
 
 When you say user mode driver, do you mean as in drivers/uio? (taking Hans
 on Cc for these)

No UIO driver like that ever reached me.

 
 Those already have generic support for memory and interrupt resources,
 maybe we can just add a common way to associate a uio device with a struct clk
 and provide a sysfs or ioctl interface to set a clock for a given device.

I don't think the UIO framework is the right place for such a thing. If it's
just this one driver that needs modification of a clock from userspace, then
a sysfs attribute could be added to that driver. If there are several drivers
that need this, then the clock framework should be extended.

Thanks,
Hans


 
   Arnd
 
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] misc: Driver for Silicon Labs Si570 and compatibles

2011-04-20 Thread Jonathan Cameron 
On 04/19/11 22:36, Guenter Roeck wrote:
 This driver adds support for Si570, Si571, Si598, and Si599
 programmable XO/VCXO.
cc'd Michael Hennerich, as I would imagine this has some similarities to the DDS
chips we have in IIO (be it a very simple one).

Guenter, what is your use case for this part?

 Signed-off-by: Guenter Roeck guenter.ro...@ericsson.com
 ---
 v2:
 - Fixed checkpatch errors and warnings
 - Moved si570.h platform data include file to include/linux/platform_data
 - Documented reset sysfs attribute
 - Added Documentation/ABI/testing/sysfs-bus-i2c-si570
 
  Documentation/ABI/testing/sysfs-bus-i2c-si570 |   21 ++
  Documentation/misc-devices/si570  |   62 
  MAINTAINERS   |7 +
  drivers/misc/Kconfig  |   10 +
  drivers/misc/Makefile |1 +
  drivers/misc/si570.c  |  398 
 +
  include/linux/platform_data/si570.h   |   23 ++
  7 files changed, 522 insertions(+), 0 deletions(-)
  create mode 100644 Documentation/ABI/testing/sysfs-bus-i2c-si570
  create mode 100644 Documentation/misc-devices/si570
  create mode 100644 drivers/misc/si570.c
  create mode 100644 include/linux/platform_data/si570.h
 
 diff --git a/Documentation/ABI/testing/sysfs-bus-i2c-si570 
 b/Documentation/ABI/testing/sysfs-bus-i2c-si570
 new file mode 100644
 index 000..78c43f7
 --- /dev/null
 +++ b/Documentation/ABI/testing/sysfs-bus-i2c-si570
 @@ -0,0 +1,21 @@
 +What:/sys/bus/i2c/devices/busnum-devaddr/frequency
 +Date:April 2011
 +Kernel version:  2.6.40 ?
 +Contact: Guenter Roeck guenter.ro...@ericsson.com
 +Description:
 + Read or write XO/VCXO frequency of the device in Hz.
 +
 + Accepted values: 10 MHz to 1400 MHz for Si570 and Si571,
 + 10 MHz to 525 MHz for Si598 and Si599.
 +Users:
 + Userspace applications interested in knowing or changing the
 + device frequency.
 +
 +What:/sys/bus/i2c/devices/busnum-devaddr/reset
 +Date:April 2011
 +Kernel Version: 2.6.40 ?
 +Contact: Guenter Roeck guenter.ro...@ericsson.com
 +Description: Writing a value other than 0 resets the device.
 + Reading always returns 0 and has no effect.
 +Users:
 + Userspace applications interested in resetting the device.
 diff --git a/Documentation/misc-devices/si570 
 b/Documentation/misc-devices/si570
 new file mode 100644
 index 000..54e32f7
 --- /dev/null
 +++ b/Documentation/misc-devices/si570
 @@ -0,0 +1,62 @@
 +Kernel driver si570
 +=
 +
 +Supported chips:
 +  * Silicon Labs Si570/Si571
 +Prefix: 'si570'
 +Addresses scanned: None (see below)
 +Datasheets:
 +http://www.silabs.com/Support%20Documents/TechnicalDocs/si570.pdf
 +  * Silicon Labs Si598/Si599
 +Prefix: 'si598'
 +Addresses scanned: None (see below)
 +Datasheets:
 + http://www.silabs.com/Support%20Documents/TechnicalDocs/si598-99.pdf
 +
 +Author: Guenter Roeck guenter.ro...@ericsson.com
 +
 +
 +Description
 +---
 +
 +The Si570/Si598 XO and Si571/Si599 VCXO provide a low-jitter clock at any
 +frequency.
 +
 +The Si570/Si571 are user-programmable to any output frequency from 10 to 945 
 MHz
 +and select frequencies to 1400 MHz with 1 ppb resolution.
 +
 +The Si598/Si599 are user programmable to any output frequency from 10 to 525 
 MHz
 +with 28 parts per trillion (ppt) resolution.
 +
 +See the datasheets for more information.
 +
 +
 +Sysfs entries
 +-
 +
 +frequency- Selected frequency
 +reset- Writing a value other than 0 resets the device
 +
 +
 +General Remarks
 +---
 +
 +The chips support all valid 7-bit I2C addresses. I2C addresses are assigned
 +during the ordering process.
 +
 +This driver does not auto-detect devices. You will have to instantiate the
 +devices explicitly. Please see Documentation/i2c/instantiating-devices for
 +details.
 +
 +
 +Platform data
 +-
 +
 +The devices can be provided with platform data to select the factory default
 +output frequency. If platform data is not specified, the driver will assume a
 +default factory output frequency of 125 MHz for Si570/Si571 and 10 MHz for
 +Si598/Si599.
 +
 +struct si570_platform_data {
 + unsigned long fout; /* Factory default output frequency */
 +};
 diff --git a/MAINTAINERS b/MAINTAINERS
 index ec36003..4c339ea 100644
 --- a/MAINTAINERS
 +++ b/MAINTAINERS
 @@ -5406,6 +5406,13 @@ L: linux-ser...@vger.kernel.org
  S:   Maintained
  F:   drivers/tty/serial
  
 +SI570 DRIVER
 +M:   Guenter Roeck guenter.ro...@ericsson.com
 +L:   linux-ker...@vger.kernel.org
 +S:   Maintained
 +F:   drivers/misc/si570.c
 +F:   include/linux/platform_data/si570.h
 +
  TIMEKEEPING, NTP
  M:   John Stultz johns...@us.ibm.com
  M:   Thomas Gleixner t...@linutronix.de
 diff --git

Re: [PATCH v2] misc: Driver for Silicon Labs Si570 and compatibles

2011-04-20 Thread Guenter Roeck 
On Wed, Apr 20, 2011 at 05:23:36AM -0400, Jonathan Cameron wrote:
 On 04/19/11 22:36, Guenter Roeck wrote:
  This driver adds support for Si570, Si571, Si598, and Si599
  programmable XO/VCXO.
 cc'd Michael Hennerich, as I would imagine this has some similarities to the 
 DDS
 chips we have in IIO (be it a very simple one).
 
 Guenter, what is your use case for this part?
 
It is used to generate a configurable serdes clock. The clock is either
left alone at its default, or configured during system startup.

What is the use case for dds devices ?

Thanks,
Guenter
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] misc: Driver for Silicon Labs Si570 and compatibles

2011-04-20 Thread Arnd Bergmann 
On Tuesday 19 April 2011, Guenter Roeck wrote:
 This driver adds support for Si570, Si571, Si598, and Si599
 programmable XO/VCXO.
 
 Signed-off-by: Guenter Roeck guenter.ro...@ericsson.com

This needs some more explanation of what the hardware is there for,
and why it's unlike everything else that we support in Linux.

We try not to have too many things in drivers/misc that are
one-off interfaces, so if the hardware is related to something
else, it should probably go into one subsystem.

My impression from readin the source code is that this is
simply a clock device that would be used in combination with
some other device in practice that consumes the clock.
If that is true, it should probably not have a user-visible
interface, but only an interface that can be used by other
kernel drivers.

Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] misc: Driver for Silicon Labs Si570 and compatibles

2011-04-20 Thread Guenter Roeck 
On Wed, Apr 20, 2011 at 12:44:30PM -0400, Arnd Bergmann wrote:
 On Tuesday 19 April 2011, Guenter Roeck wrote:
  This driver adds support for Si570, Si571, Si598, and Si599
  programmable XO/VCXO.
  
  Signed-off-by: Guenter Roeck guenter.ro...@ericsson.com
 
 This needs some more explanation of what the hardware is there for,
 and why it's unlike everything else that we support in Linux.
 
This is a generic configurable clock device. I'll be happy to add
some text such as The device can be used for any application requiring
a static or a dynamically configurable clock, such as serdes clocks.
Not sure if that would add much value, though.

Regarding unlike everything else, not sure if that is really correct.
The DDS chips Jonathan mentioned do get pretty close, and there are
other drivers providing support for clock chips, though typically more
dedicated.  ics932s401 in misc is one example, and then there are all
the tuner chips in media/common/tuners/.

 We try not to have too many things in drivers/misc that are
 one-off interfaces, so if the hardware is related to something
 else, it should probably go into one subsystem.
 
 My impression from readin the source code is that this is
 simply a clock device that would be used in combination with
 some other device in practice that consumes the clock.

Exactly. Point here is that it can be _any_ other device.

 If that is true, it should probably not have a user-visible
 interface, but only an interface that can be used by other
 kernel drivers.
 
Depends. In our case, turns out the devices consuming the clock
have user mode drivers. Lots of history there, but the chip vendors 
provide those user mode drivers, and the teams responsible for
integrating the drivers decided to not rewrite it to kernel mode drivers.
Also, for special purposes such as margining, it is necessary to control
the clock from userspace. So, for our use case, I need the user-visible
interface. I _don't_ need the kernel interface, at least not right now,
which is why I did not add it.

Browsing through the web, it seems the chip is somewhat popular with
Amateur Radio. No idea if it would ever be controlled for such a purpose
from Linux, but if so, it would also require a user configurable frequency.

If there is a better place for such a driver than misc, please let me know.

Thanks,
Guenter

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html