Re: [RFC PATCHv3 3/4] drivers/otp: convert bfin otp to generic OTP

From: Mike Frysinger
Date: Fri Mar 25 2011 - 22:56:24 EST


On Fri, Mar 25, 2011 at 22:32, Jamie Iles wrote:
> On Fri, Mar 25, 2011 at 10:11:40PM -0400, Mike Frysinger wrote:
>> >> > +static const struct otp_region_ops bfin_region_ops = {
>> >> > +    .read_word   Â= bfin_region_read_word,
>> >> > +    .write_word   = bfin_region_write_word,
>> >> > +    .get_size    = bfin_region_get_size,
>> >> > +    .get_fmt    Â= bfin_region_get_fmt,
>> >> > +    .ioctl     Â= bfin_region_ioctl,
>> >> > +};
>> >>
>> >> hmm, i just realized this stuff is per-region. Âwouldnt the
>> >> read/write/ioctl make more sense as per-device ?
>> >
>> > No, I don't think so. ÂThe file_operations are all based on the regions
>> > rather than the device so I think it makes sense to have these as region
>> > based operations. ÂWe could make them per device and pass the region as
>> > a parameter but I'm not sure that it gains us anything.
>>
>> for a device that exports more than one region, you dont need to
>> duplicate the structure. Âas you add in more regions, the amount of
>> duplication increases.
>>
>> how many regions does your device export ? Âdoes it need different
>> read/write behavior for each ?
>
> The current devices are up to 8 regions and whilst the behaviour is the
> same for each it does need to know what region it's operating on so it
> can set the redundancy correctly.
>
> We could move the read and write methods into some device ops but they'd
> need to take an otp_region as a parameter. ÂAlso, at the moment we don't
> duplicate the structure as we're just keeping a pointer to it but I'm
> happy to move these into the device ops if you have a strong preference.

i can see keeping the fmt/size in the region, but the read/write/ioctl
really look like they belong at the device level
-mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/