linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Segher Boessenkool <segher@kernel.crashing.org>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Alexandre Bounine <Alexandre.Bounine@tundra.com>,
	David Gibson <david@gibson.dropbear.id.au>,
	linuxppc-dev list <linuxppc-dev@ozlabs.org>
Subject: Re: TSI ethernet PHY question
Date: Fri, 25 May 2007 02:57:24 +0200	[thread overview]
Message-ID: <adf40b0a9d45728359da78fa45efab8a@kernel.crashing.org> (raw)
In-Reply-To: <1180051272.32247.1087.camel@localhost.localdomain>

>> You still have the same problems as Andy described where the
>> necessary workaround is not something local to phylib, but
>> needs cooperation of the ethernet code or the soc code or
>> some other platform code.
>
> If it's in the PHY device node, the ethernet driver doesn't need to be
> involved more than calling some generic helper that finds the right
> node, parses it and generates known flags.

I am not talking about the workaround for _this_ bug, but
about other PHY workarounds that _do_ need cooperation of
other devices.  How should those be described in the
device tree?

>> Since the specific bug we're talking about here is not a
>> problem with the PHY, but a miswiring on the board, I wouldn't
>> put a flag for the workaround in the phy node in the device
>> tree.  It certainly is an option though.
>
> Why ? That's the perfect place to put it in !

Only if you think the device tree is a configuration
mechanism for the OS.  Your workaround is in the PHY,
sure; but the _bug_ is in the board.

However in this case you could put a property in the
PHY node, similar things have been done before.  It's
ugly and doesn't solve any problem (it is just as much
work to parse the board model as to find this magic
property), and you *still* should pass in the flag
from the platform layer, and not have the phylib try
to handle it by itself.

>>> The problem is that of course the PHY driver will need some powerpc
>>> specific code to go fetch that.
>>
>> The ethernet driver can handle it, instead.
>
> I don't understand why you want to involve the ethernet driver in
> something that doesn't have much to do with it.

The ethernet driver is a powerpc-specific driver, that's
one thing.  Also, the workaround should be initiated by
the platform code, so has to go through the ethernet driver
(since it instantiates the phylib driver).

> A pin of the PHY is
> miswired causing something to be enabled that shouldn't be. Ok, there 
> is
> indeed the fact that the problem is partially related to the TSI
> ethernet not supporting when that PHY feature is "enabled" but still...
> it's a PHY setting, totally specific to a given PHY revision, I'm not
> sure there's much point in having it in the eth driver.

For many similar workarounds, the ethernet driver _does_ have
to cooperate in the workaround.  For some other such workarounds,
the soc code has to be involved.  Etc. etc.

You can do a quick "fix" now by doing this magic property
thing, and it sure is a *quick* fix; but later on you'll
have to do some other workarounds the proper way.  And
you'll be stuck with the property forever.  Not such a
big deal, sure; hey, I already _did_ say I'm okay with it,
right?  It's just the "wrong" thing to do ;-)


Segher

  reply	other threads:[~2007-05-25  0:57 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-05-23  7:03 TSI ethernet PHY question Benjamin Herrenschmidt
2007-05-23 13:43 ` Alexandre Bounine
2007-05-23 15:55   ` Segher Boessenkool
2007-05-23 22:52   ` Benjamin Herrenschmidt
2007-05-24 18:53     ` Andy Fleming
2007-05-24 22:51       ` Benjamin Herrenschmidt
2007-05-24 23:54         ` Segher Boessenkool
2007-05-25  0:01           ` Benjamin Herrenschmidt
2007-05-25  0:57             ` Segher Boessenkool [this message]
2007-05-25  1:53               ` Benjamin Herrenschmidt
2007-05-25 14:24                 ` Segher Boessenkool
2007-05-25  6:02               ` Paul Mackerras
2007-05-25 14:25                 ` Segher Boessenkool
2007-05-25  2:00           ` David Gibson
2007-05-25  7:35             ` Zang Roy-r61911
2007-05-25 14:17             ` Segher Boessenkool

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=adf40b0a9d45728359da78fa45efab8a@kernel.crashing.org \
    --to=segher@kernel.crashing.org \
    --cc=Alexandre.Bounine@tundra.com \
    --cc=benh@kernel.crashing.org \
    --cc=david@gibson.dropbear.id.au \
    --cc=linuxppc-dev@ozlabs.org \
    /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).