Re: [PATCH] firmware: arm_scpi: Add hardware dependencies

[Sudeep Holla <sudeep.holla@arm.com>]

On 25/01/17 15:04, Jean Delvare wrote:
> On Wed, 25 Jan 2017 14:20:53 +0000, Sudeep Holla wrote:
>> On 25/01/17 14:14, Jean Delvare wrote:
>>> On Wed, 25 Jan 2017 13:56:23 +0000, Sudeep Holla wrote:
>>>> None, hence I didn't say you are wrong ;). I am fine having the check if
>>>> it breaks for any other architecture with COMPILE_TEST.
>>>
>>> Not sure what you mean here... The purpose of COMPILE_TEST is to allow
>>> limiting the scope of a driver withing hurting the build test coverage.
>>
>> No I agree with adding COMPILE_TEST just not ARM || ARM64
> 
> That doesn't make any sense. The only purpose of COMPILE_TEST is to be
> or'd with an architecture or platform dependency. If you disagree with
> adding an architecture or platform dependency, you can't agree with
> adding COMPILE_TEST.
> 

OK. As I said I am fine with that. I just wanted to know the details.
Also different maintainer have different opinion on how to use
COMPILE_TEST. I don't have any strong opinion on that.

>>>> Also you have mentioned it fixes 8f1498c03d15, have you seen any
>>>> regression with that commit ? If so, details in the commit would be
>>>> good.
>>>
>>> Before 8f1498c03d15, the dependency on ARM_MHU made the driver only
>>> visible on ARM kernels. Since 8f1498c03d15, the driver is proposed to
>>> all, which I think isn't correct. 
>>
>> I disagree here. It depends on mailbox as we use mailbox API. And it is
>> now used on AmLogic Meson series of SoC. So it *is correct*.
> 
> I'm fine with depending on MAILBOX, I never proposed to change that.
> But Meson are ARM SoCs. So what's the problem with making the driver
> depend on ARM?
> 

So I am not saying so, just fixes tag misled me. I assumed something was
broken with that change.

>>> In that sense my proposed patch is
>>> fixing a (user-friendliness) regression. But nothing serious.
>>
>> Can you elaborate ? What's that *user-friendliness regression* ?
>> build/boot/... ? I just need more details.
> 
> From a distribution kernel maintainer point of view, having to answer
> dozens of irrelevant questions with every kernel version update is
> highly user-unfriendly. If all drivers could have proper hardware
> dependencies from the start, it would be a lot easier.
> 

I agree, but generally I have seen suggestions no to add too much
dependency if it's not required and that's why I wanted to learn the
details.

> And same holds for virtually everyone configuring his/her kernel. I'm
> building a kernel for my x86 desktop PC, "make oldconfig" shouldn't
> bother me with ARM drivers.
> 

Agreed.

> As a side note, there's no finger-pointing implied by Fixes: tags. They
> are only meant to help people backporting patches, so that they know if
> something they backported is later fixed up. It does not imply anything
> regarding how serious the problem was.

Yes, but sometimes it's taken for stable tree and I don't think patch is
really fixing anything in that particular commit. e.g. what if x86 had
MAILBOX enabled, that patch changes nothing. So definitely not stable
material.

You can drop the fixes tag and add my ack. Please post it to
arm@kernel.org, so that ARM SoC team can pick this up directly.

-- 
Regards,
Sudeep