[linux-yocto] [PATCH v2 06/11] stmicro: Add support for the STMMAC Ethernet controller family

[Darren Hart]

On 7/6/15, 9:42 AM, "Bruce Ashfield" <bruce.ashfield at windriver.com> wrote:

>On 2015-07-06 12:16 PM, Saul Wold wrote:
>> On 07/06/2015 07:18 AM, Bruce Ashfield wrote:
>>> On 2015-07-06 9:55 AM, Paul Gortmaker wrote:
>>>> [Re: [PATCH v2 06/11] stmicro: Add support for the STMMAC Ethernet
>>>> controller family] On 05/07/2015 (Sun 22:30) Saul Wold wrote:
>>>>
>>>>> On 07/05/2015 08:52 PM, Bruce Ashfield wrote:
>>>>>> On 2015-07-01 7:15 PM, Darren Hart wrote:
>>>>>>> On 7/1/15 4:06 PM, Saul Wold wrote:
>>>>>>>> This is needed for the meta-quark BSP which is used by the Galileo
>>>>>>>> Board.
>>>>>>>>
>>>>>>>
>>>>>>> Still would like to see this in features/net - or some discussion
>>>>>>> as to
>>>>>>> why not.
>>>>>>
>>>>>> Agreed. We can start a cleanup of the net* fragments with a
>>>>>> small bit of effort here. A good example for any following
>>>>>> SOCs.
>>>>>>
>>>>> I have updated my branch linux-yocto-contrib/sgw/refactor-wip with
>>>>> this change along with the rest of the refactoring of the x86/x86_64
>>>>> and x86_base changes.
>>>>>
>>>>> One of the key differences is the way we handle MTRR_SANITIZER, by
>>>>> removing the not set as Darren recommended we get the following
>>>>> difference:
>>>>>
>>>>> < # CONFIG_MTRR_SANITIZER is not set
>>>>> ---
>>>>>> CONFIG_MTRR_SANITIZER=y
>>>>>> CONFIG_MTRR_SANITIZER_ENABLE_DEFAULT=0
>>>>>> CONFIG_MTRR_SANITIZER_SPARE_REG_NR_DEFAULT=1
>>>>>
>>>>> Paul G was the person that added the not set to have it match the
>>>>> kernel.org defconfig for x86/x86_64.  Since the default is disabled
>>>>> is there any reason to continue explicitly saying not set?
>>>>
>>>> There are a couple reasons I typically do sth. like that.  One is that
>>>> it makes it explicitly clear that it was a choice and not just a "lets
>>>> go with the default", even if in this case the underlying reason was
>>>> to get better alignment with the defconfig (which is _not_ the same as
>>>> taking the defaults for everything).
>>>>
>>>> Another reason, is that if the default changes, you won't just get
>>>>swept
>>>> along for the ride without knowing what happened.  You will stay where
>>>> you were until you decide whether you consciously want to align with
>>>>the
>>>> new default.
>>>
>>> I'm also a fan of not relying on defaults for configs we care about
>>> (as we all know, we don't set them all) for the same reasons paul
>>> highlights.
>>>
>>>>
>>>> And finally, (assuming that this is set at a higher level) you will
>>>>get
>>>> a warning from the config audit about the divergence from the more
>>>> global value if a later level (in config prio) BSP decides to change
>>>>it.
>>>
>>> Yep, and even if something selects it (to change our default), we'll
>>>get
>>> a log in the configuration audit.
>>>
>>>>
>>>> Of course none of these are critical, and if we have a lot of BSPs
>>>>that
>>>> want it on, then the explicit "not set" may not make sense anymore and
>>>> hence rolling back to accepting the default to make BSP life easier
>>>>may
>>>> be the right thing to do; I don't have enough context to know, given
>>>> that I just got dragged into this discussion now.  :)
>>>
>>> Agreed as well.
>>>
>> We all got "dragged" into this as I started the refactor and Darren
>> asked the questions.  I looked at the Kconfig description for
>> MTRR_SANITIZER and related and it seems safe to enable it as default.
>>
>> Bruce, do you want me include or exclude the MTRR_SANITIZER in a v4?
>
>Let's try it as a default to 'on' for the x86 platforms.


I thought Paul made a strong case for leaving it "is not set". It wasn't
that I objected to it being "is not set", I just wanted to know the
reasoning for it. Keeping close to defconfig unless there is a specific
reason not to makes a lot of sense to me. For one thing, we'll be testing
and using what has seen the broadest usage in industry.

I suggest leaving it as "is not set", but include a comment as to why.

Thanks for the context Paul.

-- 
Darren Hart
Intel Open Source Technology Center