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: <ed3554bc-af62-78ce-a3eb-ff5f27ade6a2@ti.com>
Date:   Fri, 12 Aug 2022 10:06:05 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH v2 net-next] net: ethernet: ti: davinci_mdio: Add
 workaround for errata i2329
Content-Language: en-US
To:     Andrew Lunn <andrew@lunn.ch>
CC:     <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>,
        <pabeni@redhat.com>, <linux-omap@vger.kernel.org>,
        <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-arm-kernel@lists.infradead.org>, <kishon@ti.com>,
        <vigneshr@ti.com>
References: <20220810111345.31200-1-r-gunasekaran@ti.com>
 <YvRNpAdG7/edUEc+@lunn.ch> <9d17ab9f-1679-4af1-f85c-a538cb330d7b@ti.com>
 <YvT8ovgHz2j7yOQP@lunn.ch>
From:   Ravi Gunasekaran <r-gunasekaran@ti.com>
In-Reply-To: <YvT8ovgHz2j7yOQP@lunn.ch>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


>> There is atleast one device sh_eth, which is not configured for autosuspend
>> but uses the bit bang core in sh_mdiobb_read() and invokes regular runtime
>> PM functions.
> 
> And that is the point of moving it into the core. It would of just
> worked for you.
> 
> If you don't feel comfortable with making this unconditional, please
> put runtime pm enabled version of mdiobb_read/mdiobb_write() in the
> core and swap sh_eth and any other drivers to using them.
> 

sh_eth is not configured for autosuspend and uses only pm_runtime_put().
davinci_mdio is configured for autosuspend and it must invoke 
pm_runtime_mark_last_busy() before calling pm_runtime_put_autosuspend().
So it looks like, there needs to be a runtime PM version of 
mdiobb_read/mdiobb_write() for each pm_runtime_put_*(). As of now, it's 
only sh_eth which is currently using runtime PM and davinci_mdio would 
be the next one. So at least in this case, two variants of 
mdiobb_read/mdiobb_write() could be added at the moment. By checking 
against the dev->power.use_autosuspend flag, it is possible to support 
both via a single version.

That being said, I'm quite inclined towards the existing implementation, 
where drivers can have wrappers written around 
mdiobb_read/mdiobb_write(). But I might be failing to see the broader 
picture. If having multiple runtime PM versions of 
mdiobb_read/mdiobb_write() benefits many other future drivers, then I 
will go ahead and add the variant(s) in the bitbang core.

Please provide your views on this. Your inputs on the next course of 
action would be helpful.

-- 
Regards,
Ravi