On Wed, May 3, 2017 at 11:24 AM, Jan Kiszka <jan.kiszka@xxxxxxxxxxx> wrote: > On 2017-05-03 10:06, Andy Shevchenko wrote: >> On Wed, May 3, 2017 at 8:35 AM, Jan Kiszka <jan.kiszka@xxxxxxxxxxx> wrote: >>> On 2017-05-02 22:53, Andy Shevchenko wrote: >>>>> + ret = device_property_read_u32(&spi->dev, "ext-vin-microvolt", &val); >>>> >>>> Why not to read u16 here? >>>> >>> >>> Can I read a property with arbitrary width? Then this would simplify >>> things. >> >> device_property_read_u16(); > > By now I found out that ACPI does not care about the property type > width, only DT does. So reading it as u16 under ACPI would be fine. But > with DT, we will need a correctly sized property as well - default is u32. I'm a bit lost here. ACPI cares and DT cares about property size. You need to agreed what to use with DT people I think based on two aspects: - what existing property is using, if any - what is preferable size to use API allows to read any size. If you stick with u32 then perhaps better to change internal variable to be u32 as well. >>> Or do I have to follow how it was defined in the ACPI or device >>> tree world? >> >> For property by the way you have to either follow existing one (by >> name and meaning), or you >> you need get an Ack from DT people (Rob, for example) to introduce such. >> >> Existing one is called "vref-external". So, definitely you need to >> figure out this with DT people. > > Good point... @Rob, @Jonathon, to avoid a different naming between the > now to be introduced ACPI usage and a potential DT binding later on, > really better define a DT one now? Which name to use for such an > external vref, the one above used by spear-adc already? I forgot to mention you also need to add a binding documentation for this driver. >>>>> + .owner = THIS_MODULE, >>>> >>>> This is redundant I'm pretty sure. >>> >>> Even in 2017, drivers keep being added that carry such assignments. Can >>> you explain when it is needed and when not? Otherwise, I will leave it in. >> >> The above I'm 100% sure is not needed. It's needed only in cases when >> framework / device core doesn't do this for ya. >> >> In the above case IIRC device core does it once for all. > Then this is not consistently filtered out in new driver reviews. I can > remove it, of course. Please, remove. I guess many of new drivers just lack of proper review :-( -- With Best Regards, Andy Shevchenko -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
