netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Charles Keepax <ckeepax@opensource.cirrus.com>
To: Andrew Lunn <andrew@lunn.ch>
Cc: <nicolas.ferre@microchip.com>, <claudiu.beznea@microchip.com>,
	<davem@davemloft.net>, <kuba@kernel.org>,
	<netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] net: macb: Correct usage of MACB_CAPS_CLK_HW_CHG flag on Zynq
Date: Wed, 23 Dec 2020 19:41:58 +0000	[thread overview]
Message-ID: <20201223194158.GG9673@ediswmail.ad.cirrus.com> (raw)
In-Reply-To: <20201223192441.GH3198262@lunn.ch>

On Wed, Dec 23, 2020 at 08:24:41PM +0100, Andrew Lunn wrote:
> On Wed, Dec 23, 2020 at 06:41:44PM +0000, Charles Keepax wrote:
> > A new flag MACB_CAPS_CLK_HW_CHG was added and all callers of
> > macb_set_tx_clk were gated on the presence of this flag.
> > 
> > if (!bp->tx_clk || !(bp->caps & MACB_CAPS_CLK_HW_CHG))
> > 
> > However the flag was not added to anything other than the new
> > sama7g5_gem, turning that function call into a no op for all other
> > systems. This breaks the networking on Zynq.
> 
> I'm not sure this is the correct fix. I think the original patch might
> be broken. Look at the commit message wording:
> 
>                                                       The patch adds a new
>     capability so that macb_set_tx_clock() to not be called for IPs having
>     this capability
> 
> So MACB_CAPS_CLK_HW_CHG disables something, not enables it. So i
> suspect this if statement is wrong and needs fixing.

Hmm... good spot, hopefully the original author can comment. The
flag name reads to me as clock rate can change, the commit message
definitely implies the opposite.

So it really depends if this function was intended to be skipped
for the sama7g5 gem or emac.

Thanks,
Charles

      reply	other threads:[~2020-12-23 19:43 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-23 18:41 [PATCH] net: macb: Correct usage of MACB_CAPS_CLK_HW_CHG flag on Zynq Charles Keepax
2020-12-23 19:24 ` Andrew Lunn
2020-12-23 19:41   ` Charles Keepax [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201223194158.GG9673@ediswmail.ad.cirrus.com \
    --to=ckeepax@opensource.cirrus.com \
    --cc=andrew@lunn.ch \
    --cc=claudiu.beznea@microchip.com \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=nicolas.ferre@microchip.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).