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
