Re: similar files: fusbh200-hcd.c and fotg210-hcd.c

[Peter Senna Tschudin]

On Tue, Sep 15, 2015 at 4:33 PM, Felipe Balbi <balbi@xxxxxx> wrote:
> On Mon, Sep 14, 2015 at 07:50:02PM +0200, Peter Senna Tschudin wrote:
>> On Mon, Sep 14, 2015 at 5:01 PM, Felipe Balbi <balbi@xxxxxx> wrote:
>> > On Sat, Sep 12, 2015 at 03:14:50PM +0200, Peter Senna Tschudin wrote:
>> >> >> Should these files be consolidated? And if so how?
>> >> > if you can find an easy way, that would be a very, very welcome patch.
>> >>
>> >> Is the ideal solution to consolidate both fusbh200-hcd.c and
>> >> fotg210-hcd.c in a single module? If this is the case, how to detect
>> >> at run time which version of the hw is present? Both are registered as
>> >
>> > does it matter ? If they work the same way, why does it matter which
>> > one's running?
>>
>> I may be missing something simple, but based on a 2 page product
>> brief, fotg210 has more resources like memory. So even if the .c files
>> are _very_ similar, there are some configuration parameters that
>> differ, for example:
>>
>> fusbh200.h:
>> #define BMCSR_VBUS_OFF (1<<4)
>> #define BMCSR_INT_POLARITY (1<<3)
>>
>> fotg210.h:
>> #define OTGCSR_A_BUS_DROP (1 << 5)
>> #define OTGCSR_A_BUS_REQ (1 << 4)
>
> Can you detect that in runtime ? If you can, detect it. If you can't use
> different platform_device_id.
>
>> >> notebook (hp elitebook 840), and on a VM, even if neither has the hw
>> >> ($ sudo modprobe fusbh200-hcd). The module loads with the warning
>> >> "fusbh200_hcd should always be loaded before uhci_hcd and ohci_hcd,
>> >> not after". On another workstation running ubuntu, I could load both
>> >> modules at the same time, producing the same warning for each module.
>> >> Should the module load if the device is not present?
>> >>
>> >> Other solution for consolidation would be to create a common_code.c,
>> >> keeping both fusbh200-hcd.c and fotg210-hcd.c only with the code that
>> >> differ. Is this better than what is there now?
>> >>
>> >> Other ideas?
>> >
>> > just combine them :-p Use platform_device_id to differentiate.

Can you check the f2xx branch at:

git@xxxxxxxxxx:petersenna/linux.git

And tell me if this is the way to go for the consolidation of the two
drivers? I started with the newest driver, did code cleanup, and
started filling the new driver with parameters from the older
FUSBH200. At the moment it compiles for x86 and probably still works
for FOTG210 devices. A concrete question I have is if should I keep
making many patches for the consolidation or should I do a single big
patch with all changes? Comments are welcome.

>>
>> I'm afraid the combined version will use the correct parameters for
>> only one of the two. But I may be missing something simple. I did a
>> diff between the two files after removing white space differences, and
>> after replacing fusbh200 by fotg210 on the fusbh200 driver. The files
>> are very similar. See: http://pastebin.com/ZRY3xePv
>
> yeah, you can totally combine them. Grep the tree for examples of how to
> use platform_device_id as I mentioned.

I'll do it. Thank you.

>
> --
> balbi



-- 
Peter
--
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/