MIME-Version: 1.0
Date:   Wed, 01 Feb 2023 11:46:01 +0100
From:   Michael Walle <michael@walle.cc>
To:     =?UTF-8?Q?Rafa=C5=82_Mi=C5=82ecki?= <zajec5@gmail.com>
Cc:     devicetree@vger.kernel.org, hayashi.kunihiko@socionext.com,
        krzysztof.kozlowski+dt@linaro.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-mediatek@lists.infradead.org, matthias.bgg@gmail.com,
        mhiramat@kernel.org, rafal@milecki.pl, robh+dt@kernel.org,
        srinivas.kandagatla@linaro.org
Subject: Re: [PATCH 3/4] nvmem: mtk-efuse: replace driver with a generic MMIO
 one
In-Reply-To: <8452b341-8695-05d8-9d03-47c9aeca0ec7@gmail.com>
References: <20230201064717.18410-4-zajec5@gmail.com>
 <20230201084821.1719839-1-michael@walle.cc>
 <8452b341-8695-05d8-9d03-47c9aeca0ec7@gmail.com>
User-Agent: Roundcube Webmail/1.4.13
Message-ID: <017a17eb99ac2b2c858d27b65c5dd372@walle.cc>
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

> Before I convert brcm,nvram to NVMEM layout I need some binding & 
> driver
> providing MMIO device access. How to handle that?

I'm not arguing against having the mmio nvmem driver. But I don't
think we should sacrifice possible write access with other drivers. And
I presume write access won't be possible with your generic driver as it
probably isn't just a memcpy_toio().

It is a great fallback for some nvmem peripherals which just maps a
memory region, but doesn't replace a proper driver for an nvmem device.

What bothers me the most isn't the driver change. The driver can be
resurrected once someone will do proper write access, but the generic
"mediatek,efuse" compatible together with the comment above the older
compatible string. These imply that you should use "mediatek,efuse",
but we don't know if all mediatek efuse peripherals will be the
same - esp. for writing which is usually more complex than the reading.

nitpick btw: why not "nvmem-mmio"?

So it's either:
  (1) compatible = "mediatek,mt8173-efuse"
  (2) compatible = "mediatek,mt8173-efuse", "mmio-nvmem"

(1) will be supported any anyway for older dts and you need to add
the specific compatibles to the nvmem-mmio driver - or keep the
driver as is.

With (2) you wouldn't need to do that and the kernel can load the
proper driver if available or fall back to the nvmem-mmio one. I'd
even make that one "default y" so it will be available on future
kernels and boards can already make use of the nvmem device even
if there is no proper driver for them.

I'd prefer (2). Dunno what the dt maintainers agree.

-michael