Re: [PATCH v1 1/3] dt-bindings: mmc: Drop unused properties

Jessica Clarke
2023-09-01T18:39:32Z

On 1 Sep 2023, at 18:43, Conor Dooley <conor@kernel.org> wrote:
>
> On Fri, Sep 01, 2023 at 06:20:38PM +0100, Jessica Clarke wrote:
>> On 1 Sep 2023, at 16:42, Conor Dooley <conor@kernel.org> wrote:
>>>
>>> On Fri, Sep 01, 2023 at 10:33:13AM +0800, William Qiu wrote:
>>>>
>>>>
>>>> On 2023/8/30 16:34, Conor Dooley wrote:
>>>>> On Wed, Aug 30, 2023 at 09:29:20AM +0200, Krzysztof Kozlowski wrote:
>>>>>> On 30/08/2023 08:50, Conor Dooley wrote:
>>>>>>> On Wed, Aug 30, 2023 at 11:18:44AM +0800, William Qiu wrote:
>>>>>>>> Due to the change of tuning implementation, it's no longer necessary to
>>>>>>>> use the "starfive,sysreg" property in dts, so drop the relevant
>>>>>>>> description in dt-bindings here.
>>>>>>>
>>>>>>> How does changing your software implantation invalidate a description of
>>>>>>> the hardware?
>>>>>>>
>>>>>>
>>>>>> Which is kind of proof that this syscon was just to substitute
>>>>>> incomplete hardware description (e.g. missing clocks and phys). We
>>>>>> should have rejected it. Just like we should reject them in the future.
>>>>>
>>>>> :s I dunno what to do with this... I'm inclined to say not to remove it
>>>>> from the binding or dts at all & only change the software.
>>>>>
>>>>>> There are just few cases where syscon is reasonable. All others is just
>>>>>> laziness. It's not only starfivetech, of course. Several other
>>>>>> contributors do the same.
>>>>>
>>>>> I'm not sure if laziness is fair, lack of understanding is usually more
>>>>> likely.
>>>>
>>>> For this, I tend to keep it in binding, but remove it from required. Because
>>>> we only modify the tuning implementation, it doesn't mean that this property
>>>> need to be removed, it's just no longer be the required one.
>>>
>>> Please only remove it from required if the current driver doesn't break
>>> if the regmap is removed.
>>
>> Either way please make sure the documentation clearly states “never use
>> this, if you’re using it you’re doing it wrong, this only exists
>> because it was wrongly used in the past”. Otherwise people writing
>> drivers for other OSes will probably use it too thinking they need to.
>
> Maybe we should just delete it if the impact is going to be negligible,
> sounds like you're not using it in FreeBSD, which was part of what I was
> worried about. Guess it depends on what Emil & the distro heads think.

FreeBSD doesn’t have StarFive drivers yet; I don’t have time to write
them, and a community member has taken it upon themselves as a hobby
but is rather inexperienced and has been struggling for months. OpenBSD
has drivers, including a modified dwmmc, but doesn’t use this property
(in fact its driver doesn’t use the compatible other than to probe the
generic driver). I don’t think anyone else has a serious port; Haiku’s
the closest but also has no StarFive support.

Jess