From: Stas Sergeev <stsp-cmBhpYW9OiY@public•gmane.org>
To: Madalin-Cristian Bucur
<madalin.bucur-KZfg59tc24xl57MIdRCFDg@public•gmane.org>,
Florian Fainelli
<f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public•gmane.org>,
"netdev-u79uwXL29TY76Z2rM5mHXA@public•gmane.org"
<netdev-u79uwXL29TY76Z2rM5mHXA@public•gmane.org>,
"grant.likely-QSEj5FYQhm4dnm+yROfE0A@public•gmane.org"
<grant.likely-QSEj5FYQhm4dnm+yROfE0A@public•gmane.org>,
"robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public•gmane.org"
<robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public•gmane.org>
Cc: "devicetree-u79uwXL29TY76Z2rM5mHXA@public•gmane.org"
<devicetree-u79uwXL29TY76Z2rM5mHXA@public•gmane.org>,
"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public•gmane.org"
<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public•gmane.org>,
Liberman Igal
<Igal.Liberman-KZfg59tc24xl57MIdRCFDg@public•gmane.org>,
Stas Sergeev
<stsp-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public•gmane.org>,
"joakim.tjernlund-SNLAxHN9vbcOP4wsBPIw7w@public•gmane.org"
<joakim.tjernlund-SNLAxHN9vbcOP4wsBPIw7w@public•gmane.org>,
Shaohui Xie <Shaohui.Xie-KZfg59tc24xl57MIdRCFDg@public•gmane.org>
Subject: Re: [PATCH 0/2] of: fsl/fman: reuse the fixed node parsing code
Date: Wed, 12 Aug 2015 19:03:01 +0300 [thread overview]
Message-ID: <55CB6E35.4020808@list.ru> (raw)
In-Reply-To: <BL2PR03MB545A213D584384018CCB30CE67E0-AZ66ij2kwaa1tTsckATbNOO6mTEJWrR4XA4E9RH9d+qIuWR1G4zioA@public.gmane.org>
12.08.2015 18:27, Madalin-Cristian Bucur пишет:
>>> Then I could call only of_* functions but the end result would be the same
>>> and of_phy_register_fixed_phy() would not justify its existence that much...
>> You didn't say you wanted to obsolete the of_phy_register_fixed_phy().
>> Since it is there (and even changed by me in a way your
>> patch will likely clash), IMHO it would be better if it is used,
>> rather than copy/pasted into the driver.
> Please note I was referring to a fictional new function that would embed the call to
> fixed_phy_register(). I was not talking about some existing API, just about a new
> of_call named of_phy_register_fixed_phy() that would in the end be called by
> of_phy_register_fixed_link() and by some drivers that want to get in the middle
> of things and get a hold on status.
Hmm, and for exactly unknown reason in your pseudocode
of_phy_register_fixed_phy() doesn't take status as an argument. :)
So I didn't see its point.
If you fix your pseudo-code, you'll add the status argument,
because it is needed for fixed_phy_register() anyway.
After that, the drivers that want to provide the status, will
just use it rather than to call fixed_phy_register() directly.
And with my changes it really have even more merits to exist.
> Maybe the fact we're reviewing two patches in one thread is what makes the
> discussion less than optimal.
I guess the bugs in the pseudo-code made me to miss its point.
>>> The getter for status you suggest would be fine, but not sure how one
>>> would retrieve
>>> it from the mac_node unless we change of_phy_register_fixed_link() to
>>> return the
>>> pointer to phy (and all the drivers that use it...).
>> If you look for instance to mvneta.c, you'll find the following:
>> ---
>> err = of_phy_register_fixed_link(dn);
>> /* In the case of a fixed PHY, the DT node associated
>> * to the PHY is the Ethernet MAC DT node.
>> */
>> phy_node = of_node_get(dn);
>> ...
>> phy = of_phy_find_device(dn);
>> ---
>>
>> So the answer is: just use the same mac_node for both.
> I understand, I'll use this approach although is suboptimal imho to
Exactly!
But at least this way is used in many currently existing
drivers, while getting the fixed-link parameters directly from
DT - is a new way of circumventing the existing API.
So I'd vote for the currently existing hacks, and in fact
I already tried to start a discussion about getting rid of the
need for of_phy_find_device(), but it didn't go.
> scan the device tree again to get a phy pointer that you need just
> to get some of info that was parsed in a call you just made.
Maybe (just maybe) of_phy_register_fixed_link() could
return the phy_device pointer. At least it will solve your
problem very cheaply. But I am sure such API additions
require a separate discussion, can't be done in a context
of discussing a small fix. If you have some free time, feel
free to raise such a discussion with API extension proposals.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public•gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2015-08-12 16:03 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-05 14:42 [PATCH 0/2] of: fsl/fman: reuse the fixed node parsing code Madalin Bucur
[not found] ` <1438785745-15517-1-git-send-email-madalin.bucur-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
2015-08-05 14:42 ` [PATCH RFC 1/2] of: separate fixed link parsing from registration Madalin Bucur
2015-08-08 17:32 ` [PATCH 0/2] of: fsl/fman: reuse the fixed node parsing code Florian Fainelli
2015-08-11 16:00 ` Stas Sergeev
2015-08-11 16:33 ` Madalin-Cristian Bucur
2015-08-11 16:58 ` Stas Sergeev
2015-08-12 13:26 ` Madalin-Cristian Bucur
2015-08-12 13:58 ` Stas Sergeev
2015-08-12 14:43 ` Madalin-Cristian Bucur
2015-08-12 15:09 ` Stas Sergeev
2015-08-12 15:27 ` Madalin-Cristian Bucur
[not found] ` <BL2PR03MB545A213D584384018CCB30CE67E0-AZ66ij2kwaa1tTsckATbNOO6mTEJWrR4XA4E9RH9d+qIuWR1G4zioA@public.gmane.org>
2015-08-12 16:03 ` Stas Sergeev [this message]
2015-08-05 14:42 ` [PATCH RFC 2/2] fsl_fman: use fixed_phy_status for MEMAC Madalin Bucur
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=55CB6E35.4020808@list.ru \
--to=stsp-cmbhpyw9oiy@public$(echo .)gmane.org \
--cc=Igal.Liberman-KZfg59tc24xl57MIdRCFDg@public$(echo .)gmane.org \
--cc=Shaohui.Xie-KZfg59tc24xl57MIdRCFDg@public$(echo .)gmane.org \
--cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public$(echo .)gmane.org \
--cc=f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public$(echo .)gmane.org \
--cc=grant.likely-QSEj5FYQhm4dnm+yROfE0A@public$(echo .)gmane.org \
--cc=joakim.tjernlund-SNLAxHN9vbcOP4wsBPIw7w@public$(echo .)gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public$(echo .)gmane.org \
--cc=madalin.bucur-KZfg59tc24xl57MIdRCFDg@public$(echo .)gmane.org \
--cc=netdev-u79uwXL29TY76Z2rM5mHXA@public$(echo .)gmane.org \
--cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public$(echo .)gmane.org \
--cc=stsp-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public$(echo .)gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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