[LEDE-DEV] [PATCH] imx6: add DSA driver for MV88E6176 switch

[Tim Harvey]

On Fri, Mar 10, 2017 at 11:05 AM, Felix Fietkau <nbd at nbd.name> wrote:
> On 2017-03-10 19:58, Tim Harvey wrote:
>> On Fri, Mar 10, 2017 at 8:39 AM, Felix Fietkau <nbd at nbd.name> wrote:
>>> On 2017-03-10 17:28, Tim Harvey wrote:
>>>> On Fri, Mar 10, 2017 at 7:49 AM, Rafał Miłecki <zajec5 at gmail.com> wrote:
>>>>> On 10 March 2017 at 16:01, Tim Harvey <tharvey at gateworks.com> wrote:
>>>>>> The MV88E6176 switch is present on the GW16083 and the GW5904
>>>>>>
>>>>>> As of a5c32a1f1996f4f75504c4a9abd1c99eaa598df1 these drivers are to be
>>>>>> enabled static in per-target kernels.
>>>>>
>>>>> A standard kernel syntax may be preferred:
>>>>> commit a5c32a1f1996 ("kernel: remove switch driver kmod packages")
>>>>
>>>> I can update the commit log if the patch requires rework.
>>>>
>>>> honestly, I'm not all that happy about 'commit a5c32a1f1996 ("kernel:
>>>> remove switch driver kmod packages")' which removed selecting dsa
>>>> kernel modules because of perceived bloat but instead this puts bloat
>>>> into the static kernel of other imx6 boards that dont' have DSA. At
>>>> least before the modules were selectable.
>>> Keeping it modular is a bad trade-off, because it was adding bloat to
>>> non-imx6 platform kernel images where *no* board was using it. On many
>>> of these, bloat is a real factor, as opposed the tiny cosmetic issue
>>> that you're describing.
>>>
>>> I'm pretty sure that no imx6 board is space constrained enough that
>>> adding a few kilobytes to the kernel image actually matters in practice.
>>>
>>
>> Perhaps I'm not understanding as I don't know what you mean by
>> 'cosmetic' issue. Requiring DSA drivers be static in the imx6 kernel
>> means other imx6 boards like wandboard will have that code in their
>> kernel. Making them a configurable module means users have the choice
>> to add that module to their rootfs or leave it out. Is the issue that
>> by default the dsa modules were getting installed on systems without
>> dsa? In that case the 'board profile' (vs 'target') should have
>> enabled those and they should otherwise be defaulted off in
>> menuconfig.
> The issue was that enabling DSA as a module selects
> CONFIG_NET_SWITCHDEV, which is a bool. This causes extra code to be
> linked into the kernel image, whether the module is installed or not.
>
> Since this adds bloat to space sensitive targets, I had to choose
> between adding dependencies to kmod-dsa, making it selectable only for
> an arbitrary list of targets, or dropping it entirely and selecting it
> in the kernel config on targets that need it.
>
> I chose the second option, because I didn't see a single case that
> actually *needed* this to be modular.
>
>> Its not a big deal to me at all... like you say this is just a few KB
>> were talking about. It just seemed to me the bloat got shifted to
>> another place is all.
> That is correct. It got shifted from a place where a few kilobytes of
> bloat actually matters over to a place where it makes no practical
> difference.
>
> - Felix
>

Got it - thanks!

I'm happy to simply have this patch merged in to enable it on IMX6 targets.

Tim