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;
Date:   Thu, 1 Sep 2022 15:47:37 +0300
From:   Vladimir Oltean <olteanv@gmail.com>
To:     Oleksij Rempel <o.rempel@pengutronix.de>
Cc:     Arun.Ramadoss@microchip.com, andrew@lunn.ch,
        linux-kernel@vger.kernel.org, UNGLinuxDriver@microchip.com,
        vivien.didelot@gmail.com, san@skov.dk, linux@armlinux.org.uk,
        f.fainelli@gmail.com, kuba@kernel.org, edumazet@google.com,
        pabeni@redhat.com, netdev@vger.kernel.org,
        Woojung.Huh@microchip.com, davem@davemloft.net
Subject: Re: [Patch net-next v2 0/9] net: dsa: microchip: add support for
 phylink mac config and link up
Message-ID: <20220901124737.mrfo3fefjsn4scuy@skbuf>
References: <20220724092823.24567-1-arun.ramadoss@microchip.com>
 <20220830065533.GA18106@pengutronix.de>
 <67690ec6367c9dc6d2df720dcf98e6e332d2105b.camel@microchip.com>
 <20220830095830.flxd3fw4sqyn425m@skbuf>
 <20220830160546.GB16715@pengutronix.de>
 <20220831074324.GD16715@pengutronix.de>
 <20220831151859.ubpkt5aljrp3hiph@skbuf>
 <20220831161055.GA2479@pengutronix.de>
 <6c4666fd48ce41f84dbdad63a5cd6f4d3be25f4a.camel@microchip.com>
 <20220901112721.GB2479@pengutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220901112721.GB2479@pengutronix.de>
Precedence: bulk

On Thu, Sep 01, 2022 at 01:27:21PM +0200, Oleksij Rempel wrote:
> > The global register 0x06 responsibilities are bit 4 for 10/100mbps
> > speed selection, bit 5 for flow control and bit 6  for duplex
> > operation. Since these three are new features added during refactoring
> > I overlooked it. 
> > To fix this, either I need to return from the ksz_set_100_10mbit &
> > ksz_duplex_flowctrl function if the chip_id is ksz87xx or add
> > dev->dev_ops for this alone.  Kindly suggest on how to proceed.
> 
> I would prefer to got ops way, to clean things up.

I can't say that that one approach is better or worse than the other.
Indirect function calls are going to be more expensive than conditionals
on dev->chip_id, but we aren't in a fast path here, so it doesn't matter
too much.

Having indirect function calls will in theory help simplify the logic of
the main function, but will require good forethought for what constitutes
an atom of functionality, in a high enough level such as to abstract
switch differences. Whereas conditionals don't require thinking that far,
you put them where you need them.

Also, indirect function calls will move the bloat somewhere else. I have
seen complaints in the past about the mv88e6xxx driver's layered structure,
making it difficult to see exactly what gets done for a certain chip.

It is probable that we don't want to mix these styles too much within a
single driver, so if work has already started towards dev_ops for
everything, then dev_ops be it, I guess.

Oleksij, are you going to submit patches with your proposal?