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: <5e0276b7-2c16-13a1-29d3-1936ffc52d23@linaro.org>
Date:   Thu, 18 May 2023 16:21:53 +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 net-next v4 1/2] dt-bindings: arm: mediatek: add
 mediatek,boottrap binding
Content-Language: en-US
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
To:     Daniel Golle <daniel@makrotopia.org>, Marek Vasut <marex@denx.de>
Cc:     Andrew Lunn <andrew@lunn.ch>, devicetree@vger.kernel.org,
        netdev@vger.kernel.org, linux-mediatek@lists.infradead.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>,
        Qingfang Deng <dqfext@gmail.com>,
        SkyLake Huang <SkyLake.Huang@mediatek.com>,
        Simon Horman <simon.horman@corigine.com>
References: <cover.1683813687.git.daniel@makrotopia.org>
 <f2d447d8b836cf9584762465a784185e8fcf651f.1683813687.git.daniel@makrotopia.org>
 <55f8ac31-d81d-43de-8877-6a7fac2d37b4@lunn.ch>
 <7e8d0945-dfa9-7f61-b075-679e8a89ded9@linaro.org>
 <ZGWRHeE3CXeAnQ-5@makrotopia.org>
 <2048ed2a-ae6f-b425-38e4-4ba973e04398@linaro.org>
In-Reply-To: <2048ed2a-ae6f-b425-38e4-4ba973e04398@linaro.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 18/05/2023 09:50, Krzysztof Kozlowski wrote:
> On 18/05/2023 04:44, Daniel Golle wrote:
>> On Fri, May 12, 2023 at 08:54:36AM +0200, Krzysztof Kozlowski wrote:
>>> On 11/05/2023 17:53, Andrew Lunn wrote:
>>>> On Thu, May 11, 2023 at 04:10:20PM +0200, Daniel Golle wrote:
>>>>> The boottrap is used to read implementation details from the SoC, such
>>>>> as the polarity of LED pins. Add bindings for it as we are going to use
>>>>> it for the LEDs connected to MediaTek built-in 1GE PHYs.
>>>>
>>>> What exactly is it? Fuses? Is it memory mapped, or does it need a
>>>> driver to access it? How is it shared between its different users?
>>>
>>> Yes, looks like some efuse/OTP/nvmem, so it should probably use nvmem
>>> bindings and do not look different than other in such class.
>>
>> I've asked MediaTek and they have replied with an elaborate definition.
>> Summary:
>> The boottrap is a single 32-bit wide register at 0x1001f6f0 which can
>> be used to read back the bias of bootstrap pins from the SoC as follows:
> 
> Is it within some other address space? Register address suggests that.
> 
> In such case you should not create a device in the middle of other
> device's address space. You punched a hole in uniform address space
> which prevents creating that other device for entire space.
> 
>>
>> * bit[8]: Reference CLK source && gphy port0's LED
>> If bit[8] == 0:
>> - Reference clock source is XTRL && gphy port0's LED is pulled low on board side
>> If bit[8] == 1:
>> - Reference clock source is Oscillator && gphy port0's LED is pulled high on board side
>>
>> * bit[9]: DDR type && gphy port1's LED
>> If bit[9] == 0:
>> - DDR type is DDRx16b x2 && gphy port1's LED is pulled low on board side
>> If bit[9] == 1:
>> - DDR type is DDRx16b x1 && gphy port1's LED is pulled high on board side
>>
>> * bit[10]: gphy port2's LED
>> If bit[10] == 0:
>> - phy port2's LED is pulled low on board side
>> If bit[10] == 1:
>> - gphy port2's LED is pulled high on board side
>>
>> * bit[11]: gphy port3's LED
>> If bit[11] == 0:
>> - phy port3's LED is pulled low on board side
>> If bit[11] == 1:
>> - gphy port3's LED is pulled high on board side
>>
>> If bit[10] == 0 && bit[11] == 0:
>> - BROM will boot from SPIM-NOR
>> If bit[10] == 1 && bit[11] == 0:
>> - BROM will boot from SPIM-NAND
>> If bit[10] == 0 && bit[11] == 1:
>> - BROM will boot from eMMC
>> If bit[10] == 1 && bit[11] == 1:
>> - BROM will boot from SNFI-NAND
>>
>> The boottrap is present in many MediaTek SoCs, however, support for
>> reading it is only really needed on MT7988 due to the dual-use of some
>> bootstrap pins as PHY LEDs.
>>
>> We could say this is some kind of read-only 'syscon' node (and hence
>> use regmap driver to access it), that would make it easy but it's not
>> very accurate. Also efuse/OTP/nvmem doesn't seem accurate, though in
>> terms of software it could work just as well.
>>
>> I will update DT bindings to contain the gained insights.
> 
> If this is separate address space with one register, then boottrap
> sounds ok. If you have multiple read only registers with fused values,
> then this is efuse region, so something like nvidia,tegra20-efuse.

Please align together on some common solution. It looks like you are
solving the same problem:

https://lore.kernel.org/all/?q=%22nvmem%3A+syscon%3A+Add+syscon+backed+nvmem+driver%22

Best regards,
Krzysztof