Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <ad788d84-48ea-2fdb-607a-a8d49c8fe52c@kernel.org>
Date:   Tue, 30 May 2023 10:01:39 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [PATCH 2/3] dt-bindings: rtc: Add Mstar SSD20xD RTC devicetree
 bindings documentation
Content-Language: en-US
To:     Romain Perier <romain.perier@gmail.com>
Cc:     Alessandro Zummo <a.zummo@towertech.it>,
        Alexandre Belloni <alexandre.belloni@bootlin.com>,
        Daniel Palmer <daniel@0x0f.com>,
        Rob Herring <robh+dt@kernel.org>, linux-rtc@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org
References: <20230517144144.365631-1-romain.perier@gmail.com>
 <20230517144144.365631-3-romain.perier@gmail.com>
 <669d7b79-71a6-e1f9-8d7a-71c4b64de28d@kernel.org>
 <CABgxDoKaVip=T5=s2Gd8qpX15cLD=_0TZtQoNodK1CCf+GTYZw@mail.gmail.com>
From:   Krzysztof Kozlowski <krzk@kernel.org>
In-Reply-To: <CABgxDoKaVip=T5=s2Gd8qpX15cLD=_0TZtQoNodK1CCf+GTYZw@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 22/05/2023 13:27, Romain Perier wrote:
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - mstar,ssd20xd-rtc
>>
>> Why rtc suffix? Can it be anything else?
> 
> Well, it is the dt-bindings for an RTC block ? suppose tomorrow we
> have an ethernet block specific to the SoC SSD202D, it should be
> "mstar,ssd202d-ethernet" , how do you make
> the difference if you just put "mstar,sd202d" ? Plus a lot of rtc
> dt-bindings have this suffix (when it is not an IP name).

There are a lot of bad design choices or bugs - are you going to
implement the same mistakes because someone did it?

> This is
> exactly the case for rtc-msc313e and it was not an issue.

So that was my question - can it be anything else? There is literally no
description of the hardware... Neither in commit msg nor in description:
field in bindings.

What is SSD202D? SoC? RTC?


> 
>>
>> Missing blank line
> 
> ack
> 
>>
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  start-year: true
>>
>> Drop
>>
>> What about interrupt line?
> 
> There is currently no interrupt right now, we have not yet the irqchip
> code for handling the alarm irq of this rtc block.

So you are going to change the hardware and add the interrupt line? We
do not talk about drivers, but hardware. Whether your driver handles it
or not, matters less.

Describe the hardware, not the current implementation of one driver.


Best regards,
Krzysztof