Greetings, Am Montag, den 09.05.2016, 10:00 +0200 schrieb Peter Rosin: > Hi! > > [adding acpi people] > > On 2016-05-09 03:41, Phil Reid wrote: > > G'day Karl, > > On 7/05/2016 03:23, Karl-Heinz Schneider wrote: > >> Hi Phil > >> > >> Am Freitag, den 06.05.2016, 15:27 +0800 schrieb Phil Reid: > >>> G'day Karl > >>> On 29/04/2016 03:15, Karl-Heinz Schneider wrote: > >>>> > >>>> I have written an Kernel driver for the LTC1760 which is basically an > >>>> charger which can handle 2 batteries. Datasheet can be found at > >>>> http://www.linear.com/product/LTC1760 > >>> They're nice chips. > >>>> > >>>> However, the device has one speciality: Hence it handles two smart > >>>> batteries, which are expected to sit on I2C address 0x0b, it implements > >>>> an i2c mux. As the device does so, my driver does also (using > >>>> i2c_add_mux_adapter() call). > >>>> Further more, Linux already ships with an driver capable to talk to > >>>> these smart battery chips, namely "sbs-battery". > >>>> > >>>> I currently using device tree to bind the LTC1760 to the smbus it sits > >>>> on and further to define the i2c-lines it implements as well as the > >>>> batteries sitting on the two muxed lines. > >>>> > >>>> Would you say this approach is technically right? The LTC expects SBS > >>>> compliant batteries connected to it, which implies a standard minimal > >>>> interface. But binding the batteries via device tree gives the user the > >>>> freedom to specify a more specialized driver. > >>>> On the other hand one could argue that if the LTC is present, also > >>>> batteries are (potentially) present and the LTC driver is responsible to > >>>> read the related registers and provide proper PM attributes. Personally > >>>> I don't like to rewrite or copy code wich works just fine... > >>>> > >>> > >>> I've been writing a driver for the same chip :). > >>> My system has 2 ltc1760 for a total of 4 batteries. > >>> Haven't completed it as yet so hadn't posted, but got it talking to the batteries. > >>> I implemented an I2c mux in the driver and just attached two sbs-battery's to it in the device tree. > >>> I think the mux is the way to go, simple and reuses existing code. > >>> > >> > >> Think so too. > > I also think the muxing in ltc1760 appears to fit nicely with the > i2c-mux framework. But I only had a cursory look... > > >>> FYI, if you didn't find it there is an acpi only driver for the ltc1760 in the kernel. > >>> But I could see a way to make it work with device trees. It enumerates it's own > >>> batteries. > >> > >> Indeed I failed to find it. Looked through drivers to find something > >> similar. How is it named? > > It wrapped up in the acpi/sbs.c driver. Does specifically mention the ltc1760. > > It doesn't really do much with the charger other than use it to enumerate number of batteries > > and switch the i2c mux. > > Search for manager_present and you'll find the relevant code. > > I had a look at the acpi code you point to, and it appears to be > racy. > > It registers an attribute (alarm_attr) for every batteriy with the > "store" function set to acpi_battery_alarm_store, which calls > acpi_battery_set_alarm, which -- without any locking -- checks if > the mux is set correctly, updates the mux if not, and then writes > to the battery. > > I see other code-paths that also appear to touch the mux in > similar ways (i.e. without locking) but I didn't really look any > further than the above, which seems to be enough of a real > problem if separate users write to the alarm attr of batteries > connected to the same manager. > > The suggested fix is to register the ltc1760 mux as a real > i2c-mux, which would probably be easiest when the i2c-mux locking > update scheduled for 4.7 has landed (presently available in the > i2c for-next branch and in linux-next). Then I will take a look at this new stuff. For the device I target I'm forced to use an old 3.14 Kernel :-(. At the moment I'm doing some further tests, especially the thing with read/write failures which do happen from time to time does bother me (ltc1760 does stretch i2c clock). Once I'm happy with it I will post code. > > Cheers, > Peter Greetings Karl-Heinz -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
