[PATCH 6/6] RTC: Trivially probe for an M41T80 (#2)

[PATCH 6/6] RTC: Trivially probe for an M41T80 (#2)

[Maciej W. Rozycki]

 When probing the driver try to access the device with a read to one of
its registers and exit silently if the read fails.  This is so that boards
may register this device unconditionally and do not trigger error messages
at the bootstrap, where there is no other way to determine if an
M41T80-class RTC is actually there.

Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
---
 Please note this patch trivially depends on 
patch-2.6.26-rc1-20080505-m41t80-smbus-17 -- 5/6 of this set.

  Maciej

patch-2.6.26-rc1-20080505-m41t80-probe-18
diff -up --recursive --new-file linux-2.6.26-rc1-20080505.macro/drivers/rtc/rtc-m41t80.c linux-2.6.26-rc1-20080505/drivers/rtc/rtc-m41t80.c
--- linux-2.6.26-rc1-20080505.macro/drivers/rtc/rtc-m41t80.c	2008-05-05 02:55:40.000000000 +0000
+++ linux-2.6.26-rc1-20080505/drivers/rtc/rtc-m41t80.c	2008-05-11 00:30:54.000000000 +0000
@@ -690,10 +690,11 @@ static struct notifier_block wdt_notifie
 static int m41t80_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
-	int rc = 0;
 	struct rtc_device *rtc = NULL;
 	struct rtc_time tm;
 	struct m41t80_data *clientdata = NULL;
+	int reg;
+	int rc;
 
 	if (!i2c_check_functionality(client->adapter,
 				     I2C_FUNC_SMBUS_BYTE_DATA)) {
@@ -701,6 +702,13 @@ static int m41t80_probe(struct i2c_clien
 		goto exit;
 	}
 
+	/* Trivially check it's there; keep the result for the HT check. */
+	reg = i2c_smbus_read_byte_data(client, M41T80_REG_ALARM_HOUR);
+	if (reg < 0) {
+		rc = -ENXIO;
+		goto exit;
+	}
+
 	dev_info(&client->dev,
 		 "chip found, driver version " DRV_VERSION "\n");
 
@@ -723,11 +731,7 @@ static int m41t80_probe(struct i2c_clien
 	i2c_set_clientdata(client, clientdata);
 
 	/* Make sure HT (Halt Update) bit is cleared */
-	rc = i2c_smbus_read_byte_data(client, M41T80_REG_ALARM_HOUR);
-	if (rc < 0)
-		goto ht_err;
-
-	if (rc & M41T80_ALHOUR_HT) {
+	if (reg & M41T80_ALHOUR_HT) {
 		if (clientdata->features & M41T80_FEATURE_HT) {
 			m41t80_get_datetime(client, &tm);
 			dev_info(&client->dev, "HT bit was set!\n");
@@ -740,18 +744,18 @@ static int m41t80_probe(struct i2c_clien
 		}
 		if (i2c_smbus_write_byte_data(client,
 					      M41T80_REG_ALARM_HOUR,
-					      rc & ~M41T80_ALHOUR_HT) < 0)
+					      reg & ~M41T80_ALHOUR_HT) < 0)
 			goto ht_err;
 	}
 
 	/* Make sure ST (stop) bit is cleared */
-	rc = i2c_smbus_read_byte_data(client, M41T80_REG_SEC);
-	if (rc < 0)
+	reg = i2c_smbus_read_byte_data(client, M41T80_REG_SEC);
+	if (reg < 0)
 		goto st_err;
 
-	if (rc & M41T80_SEC_ST) {
+	if (reg & M41T80_SEC_ST) {
 		if (i2c_smbus_write_byte_data(client, M41T80_REG_SEC,
-					      rc & ~M41T80_SEC_ST) < 0)
+					      reg & ~M41T80_SEC_ST) < 0)
 			goto st_err;
 	}
 

Re: [PATCH 6/6] RTC: Trivially probe for an M41T80 (#2)

[Jean Delvare]

Hi Maciej,

On Tue, 13 May 2008 04:27:42 +0100 (BST), Maciej W. Rozycki wrote:
>  When probing the driver try to access the device with a read to one of
> its registers and exit silently if the read fails.  This is so that boards
> may register this device unconditionally and do not trigger error messages
> at the bootstrap, where there is no other way to determine if an
> M41T80-class RTC is actually there.

I don't like this. You are only supposed to declare in platform init
structures, I2C devices that you are sure are present. Relying on the
driver to not attach to the device if it is in fact not there sounds
wrong, because the I2C device will still be declared, so it's
confusing. Also, you consider that a driver silently failing to attach
is a feature, and in your specific case it may be, but for other users
it will be an annoyance: in the general case you want errors to be
clearly reported.

If you are not sure that an I2C device will be present, then you should
not declare it as part of the I2C board info, but register it later
with i2c_new_probed_device(). If this isn't possible or not convenient,
then I'd rather add a probing variant of i2c_register_board_info() (or
maybe a new flag in i2c_board_info.flags) than hack all i2c drivers to
silent failures when devices are missing.

-- 
Jean Delvare

Re: [PATCH 6/6] RTC: Trivially probe for an M41T80 (#2)

[Maciej W. Rozycki]

Hi Jean,

> I don't like this. You are only supposed to declare in platform init
> structures, I2C devices that you are sure are present. Relying on the
> driver to not attach to the device if it is in fact not there sounds
> wrong, because the I2C device will still be declared, so it's

 Well, the theory behind I2C addressing is no two devices should ever
share the same location.  Now that being a mere 7-bit field (at least with
standard devices) does not give too much of a chance for the lack of
duplication being universally true.  However within a given system it is
possible to arrange for no conflicts to be present and any given location
to be possibly associated with a single type of device only.

 The end result of an optional device being declared and later on found by
its driver not to be present should have no harm, should it?

> confusing. Also, you consider that a driver silently failing to attach
> is a feature, and in your specific case it may be, but for other users
> it will be an annoyance: in the general case you want errors to be
> clearly reported.

 The question is whether it actually is an error or not.  Assuming the
configuration has been specified correctly, there are two cases possible
-- either the device is missing or the device is faulty.  If the latter,
an error should be reported.  If the former, it should or it should not.  
Depending on whether it is expected or not.  If for example the device is
meant to be there, but it has dropped off the PCB, then clearly it is an
error condition; if it is a manufacturing option that was not included in
this particular copy of the board, then it is expected and not a problem
at all (of course in this case it is indistinguishable from a dropped off
part).

 Not reporting errors for devices believed to be missing altogether would 
be similar to what drivers for some other devices that do not report 
themselves with identifiers do.  This is for example the case with most 
ISA devices -- if pokes at I/O ports indicate the device is not there, the 
driver quits silently.  Some do report they probe though.  Please note I 
do not mean such a driver should quit with a success returned.

> If you are not sure that an I2C device will be present, then you should
> not declare it as part of the I2C board info, but register it later
> with i2c_new_probed_device(). If this isn't possible or not convenient,
> then I'd rather add a probing variant of i2c_register_board_info() (or
> maybe a new flag in i2c_board_info.flags) than hack all i2c drivers to
> silent failures when devices are missing.

 Well, the SWARM can either have a ST M41T81 or a Xicor X1241 RTC.  It is
a manufacturing option and either of these will be present and no other
device will use the same address.  The presence of either is not recorded
anywhere, because you can query the hardware directly to see which one is
there -- there is no need to duplicate this information elsewhere, the
firmware supports both and I do not think it has any hardcoded notion of
what's on-board and what's not.  Note the M41T81 has a hardwired slave
address of 0x68 -- there are no additional address select pins.  
Similarly the RTC function of the X1241 only responds to 0x6f (and its
EEPROM to 0x57).

 Then I am not sure how i2c_new_probed_device() could be used for a
baseboard device.  With an option card bearing an I2C adapter it can be
done at the time the card, including the adapter, is initialized.  With a
baseboard adapter it really begs to be in board initialization.  It cannot
be tied to i2c-sibyte.c, because it is a generic adapter driver -- the SOC
can be used in any configuration, not just the SWARM and friends.  
Perhaps it can be done with proper platform device initialization for the
SWARM, but I fear it will not happen shortly.  Hmm...

 The idea to have a flag along the lines of I2C_CLIENT_OPTION marking 
the device may or may not be there in struct i2c_board_info seems 
reasonable, perhaps better for some cases than i2c_new_probed_device() 
even, as the lack of standardisation makes device-independent probes a bit 
dangerous as already noted in the function.  Here the device driver itself 
could perform probing in a known-safe way for the given device.

 And last but not least, this whole consideration may actually not matter
anymore as I have yet to encounter a SWARM or similar board featuring an
X1241 rather than an M41T81.  While no piece of documentation mentions it
(all the board documents simply state either of the RTC chips is present)  
I think X1241 chips were used for older revisions of the board which had
serious errata in the BCM1250A SOC requiring gross changes to GCC to get a
working kernel.  These changes were never pushed upstream and the old
version of the compiler they were intended for is no longer capable of
building Linux.  Therefore chances are there is no way to get a board with
the X1241 running under Linux.  And perhaps the antiquated support for the
X1241 that we have could be entirely dropped and the M41T81 assumed as a
fixture for these boards.

 Anybody cares to comment on the last paragraph?

 Please note this change is not strictly required for the rest of the set
to operate correctly on with an M41T81-equipped board, so let's perhaps
pull this single patch out till we reach some consensus and proceed with
the rest independently.

  Maciej

Re: [PATCH 6/6] RTC: Trivially probe for an M41T80 (#2)

[Jean Delvare]

Hi Maciej,

On Wed, 14 May 2008 02:13:34 +0100 (BST), Maciej W. Rozycki wrote:
> > I don't like this. You are only supposed to declare in platform init
> > structures, I2C devices that you are sure are present. Relying on the
> > driver to not attach to the device if it is in fact not there sounds
> > wrong, because the I2C device will still be declared, so it's
> 
>  Well, the theory behind I2C addressing is no two devices should ever
> share the same location.  Now that being a mere 7-bit field (at least with
> standard devices) does not give too much of a chance for the lack of
> duplication being universally true.  However within a given system it is
> possible to arrange for no conflicts to be present and any given location
> to be possibly associated with a single type of device only.
> 
>  The end result of an optional device being declared and later on found by
> its driver not to be present should have no harm, should it?

As I wrote above: the I2C device will be listed as present forever. I
believe it's pretty confusing to have a device listed
under /sys/bus/i2c/devices which is in fact not present on the system.
On top of that, in case of modular setups with proper coldplug support,
the I2C device driver will be loaded automatically when the device is
declared, but will not be removed automatically when the probe fails,
so this is wasted memory.

> > confusing. Also, you consider that a driver silently failing to attach
> > is a feature, and in your specific case it may be, but for other users
> > it will be an annoyance: in the general case you want errors to be
> > clearly reported.
> 
>  The question is whether it actually is an error or not.  Assuming the
> configuration has been specified correctly, there are two cases possible
> -- either the device is missing or the device is faulty.  If the latter,
> an error should be reported.  If the former, it should or it should not.  
> Depending on whether it is expected or not.  If for example the device is
> meant to be there, but it has dropped off the PCB, then clearly it is an
> error condition; if it is a manufacturing option that was not included in
> this particular copy of the board, then it is expected and not a problem
> at all (of course in this case it is indistinguishable from a dropped off
> part).
> 
>  Not reporting errors for devices believed to be missing altogether would 
> be similar to what drivers for some other devices that do not report 
> themselves with identifiers do.  This is for example the case with most 
> ISA devices -- if pokes at I/O ports indicate the device is not there, the 
> driver quits silently.  Some do report they probe though.  Please note I 
> do not mean such a driver should quit with a success returned.

The comparison with ISA devices does not hold: ISA devices are created
by their own drivers (much like the legacy I2C device drivers.) Thus,
when the probe fails, they can simply delete the device they created (or
even better, they can delay the device creation until they are sure
that the device is there.) New-style I2C device drivers do not create
their devices, so they don't get a chance to delete them nor to delay
their creation.

> > If you are not sure that an I2C device will be present, then you should
> > not declare it as part of the I2C board info, but register it later
> > with i2c_new_probed_device(). If this isn't possible or not convenient,
> > then I'd rather add a probing variant of i2c_register_board_info() (or
> > maybe a new flag in i2c_board_info.flags) than hack all i2c drivers to
> > silent failures when devices are missing.
> 
>  Well, the SWARM can either have a ST M41T81 or a Xicor X1241 RTC.  It is
> a manufacturing option and either of these will be present and no other
> device will use the same address.  The presence of either is not recorded
> anywhere, because you can query the hardware directly to see which one is
> there -- there is no need to duplicate this information elsewhere, the
> firmware supports both and I do not think it has any hardcoded notion of
> what's on-board and what's not.  Note the M41T81 has a hardwired slave
> address of 0x68 -- there are no additional address select pins.  
> Similarly the RTC function of the X1241 only responds to 0x6f (and its
> EEPROM to 0x57).
> 
>  Then I am not sure how i2c_new_probed_device() could be used for a
> baseboard device.  With an option card bearing an I2C adapter it can be
> done at the time the card, including the adapter, is initialized.  With a
> baseboard adapter it really begs to be in board initialization.  It cannot
> be tied to i2c-sibyte.c, because it is a generic adapter driver -- the SOC
> can be used in any configuration, not just the SWARM and friends.  
> Perhaps it can be done with proper platform device initialization for the
> SWARM, but I fear it will not happen shortly.  Hmm...

It could certainly be implemented in terms of i2c_new_probed_device()
in i2c-sibyte, if it was a proper platform driver with proper platform
data. I admit it wouldn't be necessarily very elegant, in particular if
we have more similar cases in the future. That could still do for now,
though. Converting i2c-sibyte to a proper platform driver is needed
anyway.

>  The idea to have a flag along the lines of I2C_CLIENT_OPTION marking 
> the device may or may not be there in struct i2c_board_info seems 
> reasonable, perhaps better for some cases than i2c_new_probed_device() 
> even, as the lack of standardisation makes device-independent probes a bit 
> dangerous as already noted in the function.  Here the device driver itself 
> could perform probing in a known-safe way for the given device.

The flag in struct i2c_board_info would trigger the exact same probing
mechanism as i2c_new_probed_device() does. The whole point it to test
for the presence of the device _before_ its driver is loaded. It you
have to let the I2C device driver decide, then it's already too late:
the I2C device is created and it won't go even if the probe fails.

If said probing mechanism doesn't work properly for some devices, we'll
improve it, but it has to stay in i2c-core.

> (...)
>  Please note this change is not strictly required for the rest of the set
> to operate correctly on with an M41T81-equipped board, so let's perhaps
> pull this single patch out till we reach some consensus and proceed with
> the rest independently.

Agreed.

-- 
Jean Delvare

Re: [PATCH 6/6] RTC: Trivially probe for an M41T80 (#2)

[Maciej W. Rozycki]

Hi Jean,

> As I wrote above: the I2C device will be listed as present forever. I
> believe it's pretty confusing to have a device listed
> under /sys/bus/i2c/devices which is in fact not present on the system.

 It somehow escaped me a sysfs entry would be present even before the 
driver for this device has been loaded.  You are right, of course.

> On top of that, in case of modular setups with proper coldplug support,
> the I2C device driver will be loaded automatically when the device is
> declared, but will not be removed automatically when the probe fails,
> so this is wasted memory.

 We used to support module unloading unconditionally -- I keep forgetting
this was lifted at one point.  Personally I do not think removing the
requirement for modules to support unloading was a good decision and here
is a good example where it hurts, but obviously we have to live with
whatever consequences of that decision we have.

> The comparison with ISA devices does not hold: ISA devices are created
> by their own drivers (much like the legacy I2C device drivers.) Thus,
> when the probe fails, they can simply delete the device they created (or
> even better, they can delay the device creation until they are sure
> that the device is there.) New-style I2C device drivers do not create
> their devices, so they don't get a chance to delete them nor to delay
> their creation.

 Sometimes you may have to use a device-specific probe sequence and this
is where my comparison is right.  You cannot always escape with random
poking at the device done by the core.  For example some devices have
additional manufacturer/device ID registers -- only the respective drivers
will know which ones are acceptable.  Others may get badly hurt -- not
necessarily broken, but for example reconfigured undesirably -- as a
results of blind accesses.

 Although I agree a generic probe would work with both RTC chips under
question here.

> It could certainly be implemented in terms of i2c_new_probed_device()
> in i2c-sibyte, if it was a proper platform driver with proper platform
> data. I admit it wouldn't be necessarily very elegant, in particular if
> we have more similar cases in the future. That could still do for now,
> though. Converting i2c-sibyte to a proper platform driver is needed
> anyway.

 No question about the conversion -- it's just unlikely to happen soon.

> The flag in struct i2c_board_info would trigger the exact same probing
> mechanism as i2c_new_probed_device() does. The whole point it to test
> for the presence of the device _before_ its driver is loaded. It you
> have to let the I2C device driver decide, then it's already too late:
> the I2C device is created and it won't go even if the probe fails.

 That sounds like a limitation, although perhaps there is not enough
justification to lift it.  I can certainly imagine a flag telling the
driver: "Please check if this device is there for me [=the core] as the
complication behind that is beyond my capabilities."

> If said probing mechanism doesn't work properly for some devices, we'll
> improve it, but it has to stay in i2c-core.

 No doubt as it is good enough for many devices.

> >  Please note this change is not strictly required for the rest of the set
> > to operate correctly on with an M41T81-equipped board, so let's perhaps
> > pull this single patch out till we reach some consensus and proceed with
> > the rest independently.
> 
> Agreed.

 OK.  For now I have investigated hardware compatibility between the
M41T81 and the X1241.  The pinouts are clearly incompatible and I have had
a look at the PCB of my SWARM and it looks like there are no alternative
soldering pads nor any way to rewire the existing ones.  There is no
single dmesg archived in the Internet that would quote "swarm setup: Xicor
1241 RTC detected" either (admittedly that message got removed a short
while ago for no apparent reason, even though it proves useful here).  
Therefore I have to conclude the X1241 could only have been present on
revision 1 boards which are extremely rare and likely not supported by
Linux due to problems with GCC anymore.  Thus I have decided I will
propose to remove support for the X1241 altogether, avoiding the
complication of our I2C support with no apparent beneficiary.  

 I will follow with patch updates shortly.  If somebody who needs support
for the X1241 turns up eventually, then we can think about complications.
"Things should be kept as simple as possible, but not simpler."

  Maciej

Re: [PATCH 6/6] RTC: Trivially probe for an M41T80 (#2)

[Jean Delvare]

Hi Maciej,

On Tue, 20 May 2008 21:26:42 +0100 (BST), Maciej W. Rozycki wrote:
> > As I wrote above: the I2C device will be listed as present forever. I
> > believe it's pretty confusing to have a device listed
> > under /sys/bus/i2c/devices which is in fact not present on the system.
> 
>  It somehow escaped me a sysfs entry would be present even before the 
> driver for this device has been loaded.  You are right, of course.
> 
> > On top of that, in case of modular setups with proper coldplug support,
> > the I2C device driver will be loaded automatically when the device is
> > declared, but will not be removed automatically when the probe fails,
> > so this is wasted memory.
> 
>  We used to support module unloading unconditionally -- I keep forgetting
> this was lifted at one point.  Personally I do not think removing the
> requirement for modules to support unloading was a good decision and here
> is a good example where it hurts, but obviously we have to live with
> whatever consequences of that decision we have.

Whether the unneeded module can be removed or not, is a side problem
(which I didn't even have in mind at first, thanks for pointing it
out.) The main problem is that, even if you can remove the module, the
device you declared is still visible, even though it's not on the
system - and that's confusing.

> > The comparison with ISA devices does not hold: ISA devices are created
> > by their own drivers (much like the legacy I2C device drivers.) Thus,
> > when the probe fails, they can simply delete the device they created (or
> > even better, they can delay the device creation until they are sure
> > that the device is there.) New-style I2C device drivers do not create
> > their devices, so they don't get a chance to delete them nor to delay
> > their creation.
> 
>  Sometimes you may have to use a device-specific probe sequence and this
> is where my comparison is right.  You cannot always escape with random
> poking at the device done by the core.  For example some devices have
> additional manufacturer/device ID registers -- only the respective drivers
> will know which ones are acceptable.  Others may get badly hurt -- not
> necessarily broken, but for example reconfigured undesirably -- as a
> results of blind accesses.

What you describe here doesn't fit in the platform device declaration
model. The platform code is supposed to know what hardware is present,
with all the required configuration details. Not being certain that a
given device is there or not, as was the case for your RTC chip, is
already a variation from the standard theme. Being uncertain what chip
is at a given address would be completely off-track IMHO. That would be
much like the PC SMBus' realm where the hardware monitoring chips are
probed automatically, i.e. we do not follow the device driver model at
all. These are the legacy i2c device drivers we are trying to get rid
of at the moment, even though the plan on how to do this is still vague.

>  Although I agree a generic probe would work with both RTC chips under
> question here.
> 
> > It could certainly be implemented in terms of i2c_new_probed_device()
> > in i2c-sibyte, if it was a proper platform driver with proper platform
> > data. I admit it wouldn't be necessarily very elegant, in particular if
> > we have more similar cases in the future. That could still do for now,
> > though. Converting i2c-sibyte to a proper platform driver is needed
> > anyway.
> 
>  No question about the conversion -- it's just unlikely to happen soon.
> 
> > The flag in struct i2c_board_info would trigger the exact same probing
> > mechanism as i2c_new_probed_device() does. The whole point it to test
> > for the presence of the device _before_ its driver is loaded. It you
> > have to let the I2C device driver decide, then it's already too late:
> > the I2C device is created and it won't go even if the probe fails.
> 
>  That sounds like a limitation, although perhaps there is not enough
> justification to lift it.  I can certainly imagine a flag telling the
> driver: "Please check if this device is there for me [=the core] as the
> complication behind that is beyond my capabilities."

Again this would not fit well within the device driver model, as you
would have to load the driver before you know if you really need it.
I agree that we might have to resort to this in some cases (I have
hwmon drivers in mind as I write this.) However, my original idea was
rather to have a single module dedicated to identification of a given
type of I2C devices (say hwmon), that would act as an helper for
i2c-core, letting it instantiate the right I2C devices (in turn
triggering the loading of the appropriate device drivers.)

An alternative would be to delegate the identification task to a
user-space helper. This would however require that new-style I2C
devices can be created and removed from user-space, which isn't the
case yet.

Anyway, I don't expect embedded platforms to need this. That's more
likely to be needed only by hwmon drivers on the PC, and possibly some
old v4l drivers.

> > If said probing mechanism doesn't work properly for some devices, we'll
> > improve it, but it has to stay in i2c-core.
> 
>  No doubt as it is good enough for many devices.

-- 
Jean Delvare

Re: [PATCH 6/6] RTC: Trivially probe for an M41T80 (#2)

[Maciej W. Rozycki]

Hi Jean,

> Whether the unneeded module can be removed or not, is a side problem
> (which I didn't even have in mind at first, thanks for pointing it
> out.) The main problem is that, even if you can remove the module, the
> device you declared is still visible, even though it's not on the
> system - and that's confusing.

 No need to repeat this -- it was a side note of mine already and I agreed 
with you about the visibility of a phantom device.

> What you describe here doesn't fit in the platform device declaration
> model. The platform code is supposed to know what hardware is present,
> with all the required configuration details. Not being certain that a
> given device is there or not, as was the case for your RTC chip, is
> already a variation from the standard theme. Being uncertain what chip
> is at a given address would be completely off-track IMHO. That would be
> much like the PC SMBus' realm where the hardware monitoring chips are
> probed automatically, i.e. we do not follow the device driver model at
> all. These are the legacy i2c device drivers we are trying to get rid
> of at the moment, even though the plan on how to do this is still vague.

 Too bad for the reality it does not agree then?  While you might desire
system designers record the configuration somewhere in some software
accessible way (unless it is entirely fixed), as you can see with the
SWARM, it may not necessarily be the case and you may have to ask the
hardware directly.

> >  That sounds like a limitation, although perhaps there is not enough
> > justification to lift it.  I can certainly imagine a flag telling the
> > driver: "Please check if this device is there for me [=the core] as the
> > complication behind that is beyond my capabilities."
> 
> Again this would not fit well within the device driver model, as you
> would have to load the driver before you know if you really need it.
> I agree that we might have to resort to this in some cases (I have
> hwmon drivers in mind as I write this.) However, my original idea was
> rather to have a single module dedicated to identification of a given
> type of I2C devices (say hwmon), that would act as an helper for
> i2c-core, letting it instantiate the right I2C devices (in turn
> triggering the loading of the appropriate device drivers.)

 By all means it sounds reasonable and desirable, but I do not think this
an excuse for saying: "Let's not support this board, system, etc. because
the designers did not follow our way of thinking." -- I think if a piece
of hardware does not fit our model, then we should either adjust the model
or come up with a solution which makes the device work with as little
negative impact to the model as possible.

 Fortunately, as I mentioned previously, we can escape the dilemma for the 
SWARM for now.  I should come up with updated patches tonight.

> An alternative would be to delegate the identification task to a
> user-space helper. This would however require that new-style I2C
> devices can be created and removed from user-space, which isn't the
> case yet.

 That might make sense, although in the case of a RTC chip we have some 
desire to use it before userland is run.

> Anyway, I don't expect embedded platforms to need this. That's more
> likely to be needed only by hwmon drivers on the PC, and possibly some
> old v4l drivers.

 Hmm, an ATX-form board like the SWARM, with SDRAM and PCI slots, IDE,
Gigabit Ethernet, USB and sound ports onboard, and keyboard and VGA
support in the firmware can hardly be called embedded. ;-)  It even comes
in a desktop enclosure with drive bays, etc.  It makes quite a nice PC
based on a decent processor architecture. :-)  The only difference making
it a bit more embedded is most of the functionality is implemented in the
SOC leaving off-chip features mainly to a bunch of I2C peripherals and
then boring stuff like line drivers and passive components. ;-)

 I agree for truly embedded systems it should not matter as I would expect
them to come in a fixed configuration.

  Maciej

Re: [PATCH 6/6] RTC: Trivially probe for an M41T80 (#2)

[David Brownell]

On Tuesday 13 May 2008, Maciej W. Rozycki wrote:
> Then I am not sure how i2c_new_probed_device() could be used for a
> baseboard device.  With an option card bearing an I2C adapter it can be
> done at the time the card, including the adapter, is initialized.  With a
> baseboard adapter it really begs to be in board initialization.  

One idiom to consider is callbacks (or notifications) when the
relevant i2c_adapter is registered.

Some of the GPIO drivers do that, so that they can hook up the
relevant devices or options.  Example, when an pcf8574 expander
is used to drive LEDs, the "this chip got registered" callback
can register the relevant "leds-gpio" platform device.  Or when
it's used to switch power to other devices, sometimes they must
defer registration till then too (power them up first, *then* it
makes sense to register them).

This could handle that "board has this I2C RTC -or- that one"
case pretty cleanly; but then, so would having a "probe first"
flag that's specific to the i2c_board_info.  (So long as the
probe logic could distinguish the devices... which will not in
the most general case work from i2c-core code.)

- Dave