LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Heiner Kallweit <hkallweit1@gmail.com>
Cc: Trent Piepho <tpiepho@impinj.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"f.fainelli@gmail.com" <f.fainelli@gmail.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>
Subject: Re: [PATCH 5/5] net: phy: dp83867: Use unsigned variables to store unsigned properties
Date: Mon, 13 May 2019 22:46:41 +0200	[thread overview]
Message-ID: <20190513204641.GA12345@lunn.ch> (raw)
In-Reply-To: <b246b18d-5523-7b8b-9cd0-b8ccb8a511e9@gmail.com>

> > Perhaps you could tell me if the approach I've taken in patch 3, 
> > "Add ability to disable output clock", and patch 4, "Disable tx/rx
> > delay when not configured", are considered acceptable?  I can conceive
> > of arguments for alternate approaches.  I would like to add support for
> >  these into u-boot too, but typically u-boot follows the kernel DT
> > bindings, so I want to finalize the kernel DT semantics before sending
> > patches to u-boot.
> > 
> I lack experience with these TI PHY's. Maybe Andrew or Florian can advise.

Hi Trent

I already deleted the patches. For patch 3:

+ 	  if (dp83867->clk_output_sel > DP83867_CLK_O_SEL_REF_CLK &&
+	         dp83867->clk_output_sel != DP83867_CLK_O_SEL_OFF) {
+		 	phydev_err(phydev, "ti,clk-output-sel value %u out of range\n",
+				   dp83867->clk_output_sel);
+			return -EINVAL;
+													       }

This last bit looks odd. If it is not OFF, it is invalid?

Are there any in tree users of DP83867_CLK_O_SEL_REF_CLK? We have to
be careful changing its meaning. But if nobody is actually using it...

Patch 4:

This is harder. Ideally we want to fix this. At some point, somebody
is going to want 'rgmii' to actually mean 'rgmii', because that is
what their hardware needs.

Could you add a WARN_ON() for 'rgmii' but the PHY is actually adding a
delay? And add a comment about setting the correct thing in device
tree?  Hopefully we will then get patches correcting DT blobs. And if
we later do need to fix 'rgmii', we will break less board.

> >>> Please note that net-next is closed currently. Please resubmit the
> >>> patches once it's open again, and please annotate them properly
> >>> with net-next.
> > 
> > Sorry, didn't know about this policy.  Been years since I've submitted
> > net patches.  Is there a description somewhere of how this is done? 
> > Googling net-next wasn't helpful.  I gather new patches are only
> > allowed when the kernel merge window is open?  And they can't be queued
> > on patchwork or a topic branch until this happens?

You can post patches while it is closed for review, but add "RFC" in
the subject so it is clear you just want comments. You will still need
to resubmit once it opens up again.

    Andrew

  reply	other threads:[~2019-05-13 20:46 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-10 21:46 [PATCH 1/5] dt-bindings: phy: dp83867: Describe how driver behaves w.r.t rgmii delay Trent Piepho
2019-05-10 21:46 ` [PATCH 2/5] dt-bindings: phy: dp83867: Add documentation for disabling clock output Trent Piepho
2019-05-10 21:46 ` [PATCH 3/5] net: phy: dp83867: Add ability to disable output clock Trent Piepho
2019-05-10 21:46 ` [PATCH 4/5] net: phy: dp83867: Disable tx/rx delay when not configured Trent Piepho
2019-05-10 21:46 ` [PATCH 5/5] net: phy: dp83867: Use unsigned variables to store unsigned properties Trent Piepho
2019-05-11 10:41   ` Heiner Kallweit
2019-05-11 12:32     ` Heiner Kallweit
2019-05-13 19:58       ` Trent Piepho
2019-05-13 20:12         ` Heiner Kallweit
2019-05-13 20:46           ` Andrew Lunn [this message]
2019-05-13 21:26             ` Trent Piepho
2019-05-13 21:43               ` Andrew Lunn
2019-05-14 15:37                 ` Trent Piepho

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=20190513204641.GA12345@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=devicetree@vger.kernel.org \
    --cc=f.fainelli@gmail.com \
    --cc=hkallweit1@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=tpiepho@impinj.com \
    --subject='Re: [PATCH 5/5] net: phy: dp83867: Use unsigned variables to store unsigned properties' \
    /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

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).