From: Chris Zhong <zyw@rock-chips.com>
To: Guenter Roeck <groeck@google.com>
Cc: "Douglas Anderson" <dianders@chromium.org>,
"Tomasz Figa" <tfiga@chromium.org>,
"Heiko Stübner" <heiko@sntech.de>,
yzq@rock-chips.com, "Guenter Roeck" <groeck@chromium.org>,
linux-rockchip@lists.infradead.org,
"Kever Yang" <kever.yang@rock-chips.com>,
"Kishon Vijay Abraham I" <kishon@ti.com>,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
Subject: Re: [v2 PATCH 2/4] phy: Add USB Type-C PHY driver for rk3399
Date: Mon, 20 Jun 2016 13:57:16 +0800 [thread overview]
Message-ID: <576785BC.4070904@rock-chips.com> (raw)
In-Reply-To: <CABXOdTd+U51KY7P-Gj6TySxoAZpJY=zcXs_4_9MVD1gnmnFH0w@mail.gmail.com>
Hi Guenter
On 06/18/2016 11:45 PM, Guenter Roeck wrote:
> Hi Chris,
>
> On Mon, Jun 13, 2016 at 2:39 AM, Chris Zhong <zyw@rock-chips.com> wrote:
>> Add a PHY provider driver for the rk3399 SoC Type-c PHY. The USB
>> Type-C PHY is designed to support the USB3 and DP applications. The
>> PHY basically has two main components: USB3 and DisplyPort. USB3
>> operates in SuperSpeed mode and the DP can operate at RBR, HBR and
>> HBR2 data rates.
>>
> I started integrating the driver with our code.
> Doing so, I realized a problem in the way you are using extcon.
>
> [ ... ]
>
>> +
>> +static int tcphy_pd_event(struct notifier_block *nb,
>> + unsigned long event, void *priv)
>> +{
>> + struct rockchip_typec_phy *tcphy;
>> + struct extcon_dev *edev = priv;
>> + int value = edev->state;
>> + int mode;
>> + u8 is_plugged, dfp;
>> +
>> + tcphy = container_of(nb, struct rockchip_typec_phy, event_nb);
>> +
>> + is_plugged = GET_PLUGGED(value);
>> + tcphy->flip = GET_FLIP(value);
>> + dfp = GET_DFP(value);
>> + tcphy->map = GET_PIN_MAP(value);
>> +
> I don't think it is a good idea to use the extcon 'state' field like
> this. I don't even think it is possible.
>
> The state is supposed to be a bit mask, each bit indicating if a
> specific connector (or functionality) on the cable is attached. The
> extcon notifier code walks through this bit mask and determines based
> on changed bits if the notifier should be called. So the notifier in
> this case would only be called if bit 1 (EXTCON_USB) of 'state' has
> changed, but not if one of the other bits has changed. One would have
> to define 32 "virtual" cables, one for each bit, for this to work, and
> then you would have to register a notifier for each of the bits. That
> would not really make sense.
>
> Of course, that makes using the extcon notifier quite useless for our
> purpose, since we need the callback not only if a cable has been
> attached or deattached, but also if some secondary state changes. I
> don't really know myself how to solve the problem; I'll need to think
> about it some more. Maybe we can add a callback into the type-c
> infrastructure code and somehow tie into that code, but I don't know
> yet if that is feasible either.
>
> Guenter
>
>
Yes, currently, we can get the notification only when bit 0 change.
So the phy driver can know plug/unplug event.
if we need more trigger, how about set the BIT 0 for changed flag.
state = extcon_get_cable_state
state = ~state | is_plugged | flip | dfp | map
extcon_set_state(state)
next prev parent reply other threads:[~2016-06-20 5:58 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-13 9:39 [v2 PATCH 0/4] Rockchip Type-C and DispplayPort driver Chris Zhong
2016-06-13 9:39 ` [v2 PATCH 1/4] Documentation: bindings: add dt doc for Rockchip USB Type-C PHY Chris Zhong
2016-06-14 22:51 ` Rob Herring
2016-06-15 22:11 ` Heiko Stuebner
2016-06-16 0:31 ` Chris Zhong
2016-06-16 7:49 ` Tomasz Figa
2016-06-16 8:54 ` Heiko Stübner
2016-06-13 9:39 ` [v2 PATCH 2/4] phy: Add USB Type-C PHY driver for rk3399 Chris Zhong
2016-06-16 12:48 ` Kever Yang
2016-06-17 12:54 ` Kishon Vijay Abraham I
2016-06-20 0:32 ` Chris Zhong
2016-06-17 16:06 ` Tomasz Figa
2016-06-20 7:59 ` Chris Zhong
2016-06-18 15:45 ` Guenter Roeck
2016-06-20 5:57 ` Chris Zhong [this message]
2016-06-13 9:39 ` [v2 PATCH 3/4] Documentation: bindings: add dt documentation for cdn DP controller Chris Zhong
2016-06-14 23:12 ` Rob Herring
2016-06-13 9:39 ` [v2 PATCH 4/4] drm/rockchip: cdn-dp: add cdn DP support for rk3399 Chris Zhong
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=576785BC.4070904@rock-chips.com \
--to=zyw@rock-chips.com \
--cc=dianders@chromium.org \
--cc=groeck@chromium.org \
--cc=groeck@google.com \
--cc=heiko@sntech.de \
--cc=kever.yang@rock-chips.com \
--cc=kishon@ti.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=tfiga@chromium.org \
--cc=yzq@rock-chips.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).