Date:   Tue, 7 Mar 2023 15:00:30 +0000
From:   "Russell King (Oracle)" <linux@armlinux.org.uk>
To:     Andrew Lunn <andrew@lunn.ch>
Cc:     Vladimir Oltean <olteanv@gmail.com>,
        Michael Walle <michael@walle.cc>, sean.anderson@seco.com,
        davem@davemloft.net, edumazet@google.com, f.fainelli@gmail.com,
        hkallweit1@gmail.com, kuba@kernel.org,
        linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
        pabeni@redhat.com, tobias@waldekranz.com
Subject: Re: [PATCH net-next] net: mdio: Add netlink interface
Message-ID: <ZAdRjo3wXmVigPAC@shell.armlinux.org.uk>
References: <20230306204517.1953122-1-sean.anderson@seco.com>
 <20230307112307.777207-1-michael@walle.cc>
 <684c859a-02e2-4652-9a40-9607410f95e6@lunn.ch>
 <20230307140535.32ldprkyblpmicjg@skbuf>
 <7013dea3-a026-4a0c-81e0-7ebe6f708e39@lunn.ch>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <7013dea3-a026-4a0c-81e0-7ebe6f708e39@lunn.ch>
Sender: Russell King (Oracle) <linux@armlinux.org.uk>
Precedence: bulk

On Tue, Mar 07, 2023 at 03:33:20PM +0100, Andrew Lunn wrote:
> > - Atomic (why only atomic?) (read) access to paged registers
> 
> I would say 'atomic' is wrong, you cannot access paged registers at
> all.
> 
> > are we ok with the implications?
> 
> I am. Anybody doing this level of debugging should be able to
> recompile the kernel to enable write support. It does limit debugging
> in field, where maybe you cannot recompile the kernel, but to me, that
> is a reasonable trade off.

However, it should be pointed out that disabling write support means
that one can not even read paged registers through this interface.

That leads me to question the whole point of this, because as far as I
can see, the only thing this interface would offer with writes disabled
is the ability to read several non-paged registers consectively while
holding the mdio bus lock. Apart from that, with writes disabled, it
appears to offer nothing over the existing MII ioctls.

With writes enabled, then yes, it offers a slightly better interface
to be able to perform multiple accesses while holding the bus lock.

In that regard, is there any point to having the configuration option
to control whether writes are supported, or is it just better to have
an option to enable/disable the whole interface, and taint the kernel
on *any* use of this interface?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!