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

[Felix Fietkau]

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