[lm-sensors] dme1737 / sch311x hybrid driver

[lm-sensors] dme1737 / sch311x hybrid driver

[Juerg Haefliger]

Hi all,

I was playing around with adding support for the sch311x to the
dme1737 driver but I'm not so sure anymore how to go about it. For
those unfamiliar with the 2 chips, both chips implement the same
(almost but very close) HW monitoring capabilities but the dme1737
only allows access via SMBus while the sch311x only supports the LPC
interface. There are multiple ways to go about this:

1) 2 separate drivers. An i2c driver for the dme1737 and a platform
driver for the sch311x. They would only differ in the way they access
the HW, all the rest would be identical and could be shared.

2) 1 single driver that either registers itself as an i2c client or a
platform driver, depending on which chip is detected.

3) 1 single driver that registers itself as an i2c client independent
of the chip found.

While option 1) would be preferred, I kind of shy away from the amount
of work/changes necessary to implement it. I started of with option 2)
but it turned out to be bigger than expected. Lots of code for
registering/adding the platform stuff to support a chip that doesn't
seem to be very popular. It blows up the code and makes it pretty
ugly. I'm almost tempted to go with option 1)...

Not sure if option 3) is feasible. Sounds a little bit hacky and I
don't know if it's even possible to register a 'bogus' i2c client or
if it would break something somewhere that I'm not aware of.

Oh and option 2) and 3) of course require an i2c bus driver to be
present and loaded. So in case someone has a system with a sch311x
that doesn't have a supported i2c controller, he/she couldn't load the
dme1737 driver... Not very nice...

Does anybody have any suggestions or comments or other ideas?

Thanks
...juerg

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] dme1737 / sch311x hybrid driver

[Hans de Goede]

Juerg Haefliger wrote:
> Hi all,
> 
> I was playing around with adding support for the sch311x to the
> dme1737 driver but I'm not so sure anymore how to go about it. For
> those unfamiliar with the 2 chips, both chips implement the same
> (almost but very close) HW monitoring capabilities but the dme1737
> only allows access via SMBus while the sch311x only supports the LPC
> interface. There are multiple ways to go about this:
> 
> 1) 2 separate drivers. An i2c driver for the dme1737 and a platform
> driver for the sch311x. They would only differ in the way they access
> the HW, all the rest would be identical and could be shared.
> 
> 2) 1 single driver that either registers itself as an i2c client or a
> platform driver, depending on which chip is detected.
> 
> 3) 1 single driver that registers itself as an i2c client independent
> of the chip found.
> 
> While option 1) would be preferred, I kind of shy away from the amount
> of work/changes necessary to implement it. I started of with option 2)
> but it turned out to be bigger than expected. Lots of code for
> registering/adding the platform stuff to support a chip that doesn't
> seem to be very popular. It blows up the code and makes it pretty
> ugly. I'm almost tempted to go with option 1)...
> 
> Not sure if option 3) is feasible. Sounds a little bit hacky and I
> don't know if it's even possible to register a 'bogus' i2c client or
> if it would break something somewhere that I'm not aware of.
> 
> Oh and option 2) and 3) of course require an i2c bus driver to be
> present and loaded. So in case someone has a system with a sch311x
> that doesn't have a supported i2c controller, he/she couldn't load the
> dme1737 driver... Not very nice...
> 
> Does anybody have any suggestions or comments or other ideas?
> 

While I was looking at writing a driver for an intel motherboard superio chip 
some time ago, I had the same problem, since this superio chip could be 
connected to either smbus or LPC. I then came up with the plan to first probe 
through LPC and then depending on the configuration (which can always be read 
through LPC in this case) register either an platfrom or a i2c client driver 
and matching device. Won't that work here?

Regards,

Hans


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] dme1737 / sch311x hybrid driver

[Juerg Haefliger]

Hi Hans,

On 6/28/07, Hans de Goede <j.w.r.degoede@hhs.nl> wrote:
> Juerg Haefliger wrote:
> > Hi all,
> >
> > I was playing around with adding support for the sch311x to the
> > dme1737 driver but I'm not so sure anymore how to go about it. For
> > those unfamiliar with the 2 chips, both chips implement the same
> > (almost but very close) HW monitoring capabilities but the dme1737
> > only allows access via SMBus while the sch311x only supports the LPC
> > interface. There are multiple ways to go about this:
> >
> > 1) 2 separate drivers. An i2c driver for the dme1737 and a platform
> > driver for the sch311x. They would only differ in the way they access
> > the HW, all the rest would be identical and could be shared.
> >
> > 2) 1 single driver that either registers itself as an i2c client or a
> > platform driver, depending on which chip is detected.
> >
> > 3) 1 single driver that registers itself as an i2c client independent
> > of the chip found.
> >
> > While option 1) would be preferred, I kind of shy away from the amount
> > of work/changes necessary to implement it. I started of with option 2)
> > but it turned out to be bigger than expected. Lots of code for
> > registering/adding the platform stuff to support a chip that doesn't
> > seem to be very popular. It blows up the code and makes it pretty
> > ugly. I'm almost tempted to go with option 1)...
> >
> > Not sure if option 3) is feasible. Sounds a little bit hacky and I
> > don't know if it's even possible to register a 'bogus' i2c client or
> > if it would break something somewhere that I'm not aware of.
> >
> > Oh and option 2) and 3) of course require an i2c bus driver to be
> > present and loaded. So in case someone has a system with a sch311x
> > that doesn't have a supported i2c controller, he/she couldn't load the
> > dme1737 driver... Not very nice...
> >
> > Does anybody have any suggestions or comments or other ideas?
> >
>
> While I was looking at writing a driver for an intel motherboard superio chip
> some time ago, I had the same problem, since this superio chip could be
> connected to either smbus or LPC. I then came up with the plan to first probe
> through LPC and then depending on the configuration (which can always be read
> through LPC in this case) register either an platfrom or a i2c client driver
> and matching device. Won't that work here?

Well yes it would. That's in fact what I started doing. I just think
it's kind of ugly to have 2 sets of device register/unregister methods
in a single driver. I thought I'd run it by the list before spending
more time on it.

...juerg


> Regards,
>
> Hans
>
>

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] dme1737 / sch311x hybrid driver

[Jean Delvare]

Hi Juerg,

On Thu, 28 Jun 2007 11:28:18 -0700, Juerg Haefliger wrote:
> Hi all,
> 
> I was playing around with adding support for the sch311x to the
> dme1737 driver but I'm not so sure anymore how to go about it. For
> those unfamiliar with the 2 chips, both chips implement the same
> (almost but very close) HW monitoring capabilities but the dme1737
> only allows access via SMBus while the sch311x only supports the LPC
> interface. There are multiple ways to go about this:
> 
> 1) 2 separate drivers. An i2c driver for the dme1737 and a platform
> driver for the sch311x. They would only differ in the way they access
> the HW, all the rest would be identical and could be shared.
> 
> 2) 1 single driver that either registers itself as an i2c client or a
> platform driver, depending on which chip is detected.
> 
> 3) 1 single driver that registers itself as an i2c client independent
> of the chip found.
> 
> While option 1) would be preferred, I kind of shy away from the amount
> of work/changes necessary to implement it. I started of with option 2)
> but it turned out to be bigger than expected. Lots of code for
> registering/adding the platform stuff to support a chip that doesn't
> seem to be very popular. It blows up the code and makes it pretty
> ugly. I'm almost tempted to go with option 1)...

Option 1) can't possibly be less code than option 2). It may be a
little less ugly, but not that much.

> Not sure if option 3) is feasible. Sounds a little bit hacky and I
> don't know if it's even possible to register a 'bogus' i2c client or
> if it would break something somewhere that I'm not aware of.

Option 3) has a name: i2c-isa. It took me years to get rid of it, and
I'm just there now. If you implement something similar again, I'll hate
you forever ;) So just forget about it, it's not going to happen.

> Oh and option 2) and 3) of course require an i2c bus driver to be
> present and loaded. So in case someone has a system with a sch311x
> that doesn't have a supported i2c controller, he/she couldn't load the
> dme1737 driver... Not very nice...

Not true. Options 2) requires i2c-core to be loaded (due to symbol
dependencies) but that's about it. 

> Does anybody have any suggestions or comments or other ideas?

I'd go for option 2) because we already have such drivers (lm78 and
w83781d).

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] dme1737 / sch311x hybrid driver

[Juerg Haefliger]

Hi Jean,

On 6/29/07, Jean Delvare <khali@linux-fr.org> wrote:
> Hi Juerg,
>
> On Thu, 28 Jun 2007 11:28:18 -0700, Juerg Haefliger wrote:
> > Hi all,
> >
> > I was playing around with adding support for the sch311x to the
> > dme1737 driver but I'm not so sure anymore how to go about it. For
> > those unfamiliar with the 2 chips, both chips implement the same
> > (almost but very close) HW monitoring capabilities but the dme1737
> > only allows access via SMBus while the sch311x only supports the LPC
> > interface. There are multiple ways to go about this:
> >
> > 1) 2 separate drivers. An i2c driver for the dme1737 and a platform
> > driver for the sch311x. They would only differ in the way they access
> > the HW, all the rest would be identical and could be shared.
> >
> > 2) 1 single driver that either registers itself as an i2c client or a
> > platform driver, depending on which chip is detected.
> >
> > 3) 1 single driver that registers itself as an i2c client independent
> > of the chip found.
> >
> > While option 1) would be preferred, I kind of shy away from the amount
> > of work/changes necessary to implement it. I started of with option 2)
> > but it turned out to be bigger than expected. Lots of code for
> > registering/adding the platform stuff to support a chip that doesn't
> > seem to be very popular. It blows up the code and makes it pretty
> > ugly. I'm almost tempted to go with option 1)...
>
> Option 1) can't possibly be less code than option 2). It may be a
> little less ugly, but not that much.
>
> > Not sure if option 3) is feasible. Sounds a little bit hacky and I
> > don't know if it's even possible to register a 'bogus' i2c client or
> > if it would break something somewhere that I'm not aware of.
>
> Option 3) has a name: i2c-isa. It took me years to get rid of it, and
> I'm just there now. If you implement something similar again, I'll hate
> you forever ;) So just forget about it, it's not going to happen.

:-) I knew about i2c-isa and its status. I was not planing on using
it. I don't know, I thought it might be possible to just register the
i2c client somehow without having an actual device behind it.


> > Oh and option 2) and 3) of course require an i2c bus driver to be
> > present and loaded. So in case someone has a system with a sch311x
> > that doesn't have a supported i2c controller, he/she couldn't load the
> > dme1737 driver... Not very nice...
>
> Not true. Options 2) requires i2c-core to be loaded (due to symbol
> dependencies) but that's about it.

OK, good.


> > Does anybody have any suggestions or comments or other ideas?
>
> I'd go for option 2) because we already have such drivers (lm78 and
> w83781d).

OK I see it now. I used to look at the old versions that still had the
i2c-isa stuff.
Good, I'll do the same thing then.

Thanks
...juerg


> --
> Jean Delvare
>

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] dme1737 / sch311x hybrid driver

[Jean Delvare]

On Fri, 29 Jun 2007 13:25:56 -0700, Juerg Haefliger wrote:
> On 6/29/07, Jean Delvare <khali@linux-fr.org> wrote:
> > On Thu, 28 Jun 2007 11:28:18 -0700, Juerg Haefliger wrote:
> > > Not sure if option 3) is feasible. Sounds a little bit hacky and I
> > > don't know if it's even possible to register a 'bogus' i2c client or
> > > if it would break something somewhere that I'm not aware of.
> >
> > Option 3) has a name: i2c-isa. It took me years to get rid of it, and
> > I'm just there now. If you implement something similar again, I'll hate
> > you forever ;) So just forget about it, it's not going to happen.
> 
> :-) I knew about i2c-isa and its status. I was not planing on using
> it. I don't know, I thought it might be possible to just register the
> i2c client somehow without having an actual device behind it.

Possible, certainly. Acceptable, no ;-)

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors