Stephen Warren wrote: > On 05/16/2014 10:21 AM, Nick Dyer wrote: >> Thanks for this. Would you be happy for me to pick these changes up and >> include them along with the other work I am sending to Dmitry? I am just >> beginning to do various updates to the whole series, one of the things I >> need to sort out is the device tree support. > > That would be fine. I assume you'd only take the 2 Atmel driver patches. > I'll send the Tegra patches separately once the driver is merged. Great! Dmitry has merged some of the patches I sent now, so I'm just working on updating to take account of that and adding the device tree changes, and taking account of a couple of other review comments. > One thing I wasn't really sure about: With your latest patches, it seems > like the bootloader address is auto-calculated from the application > address. As such, do I still need separate struct i2c_device for the > application and bootloader I2C addresses? If not, we should remove the > atmel,mxt-tp-bootloader from those patches. If so, I need to add the > second DT node back into the Tegra DT. Either way, it might be > preferable if we only had 1 node in DT, and the driver automatically > handled the two separate I2C addresses. Currently you shouldn't need the extra node for the bootloader, it should figure it out itself. I think in the future, the driver should register the bootloader address with i2c_new_dummy() to prevent it being bound to a different driver. >> I will need to add device tree parameters for the touchscreen as well as >> the touchpad, of course. >> >> By the way, the driver should work without any firmware file and just use >> the firmware and configuration from NVRAM - request_firmware() returns a >> failure and it continues through mxt_initialize(). > > Hmmm. I couldn't get that to work after applying the patches you posted. > However, it did with what's already in linux-next plus the patches I sent. I will test this on my setup and see if I can figure out what is causing the problem. >> In a later patch in my long series, I make the MXT_CFG_NAME configurable >> from platform data (because you may have multiple devices needing different >> configs), and leaving it null means the call to request_firmware() is skipped. > > It'd be preferable to automatically derive the firmware name from the > device type (touchscreen/touchpad) or some other parameter that can be > calculated at run-time, or queried from HW (e.g. version # from > bootloader?). I'm not sure that putting firmware filenames in DT is a > good idea, but perhaps that would work. Deriving firmware filename from > the DT compatible value would work best. Yes, I was planning to allow the firmware filename to be specified in DT. I think that coming up with a scheme to automatically derive it would fall down in various corner cases (as an example, you might have two devices which have the same family/variant IDs but require different firmwares), so it gives maximum flexibility to not dictate the naming policy. > If different HW needs different > firmware, it should probably have a different compatible value in DT. There are literally hundreds of different combinations of hardware/firmware supported by this driver. Trying to generate a comprehensive list would be a never-ending task to support. I don't think this is a good idea. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
