Re: [PATCH 0/3] Implement role-switch notifications from dwc3-drd to dwc3-qcom

From: Felipe Balbi <balbi@kernel.org>
To: Rob Herring <robh+dt@kernel.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>,
	Peter Chen <peter.chen@kernel.org>,
	Bryan O'Donoghue <bryan.odonoghue@linaro.org>,
	"Gross, Andy" <agross@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jack Pham <jackp@codeaurora.org>,
	Wesley Cheng <wcheng@codeaurora.org>,
	Linux USB List <linux-usb@vger.kernel.org>,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>
Subject: Re: [PATCH 0/3] Implement role-switch notifications from dwc3-drd to dwc3-qcom
Date: Wed, 25 Aug 2021 18:20:59 +0300	[thread overview]
Message-ID: <87r1eha6tv.fsf@kernel.org> (raw)
In-Reply-To: <CAL_JsqLxm6P0KyYtUFvB5otR3BUkJN7dN89QixeWh67WoDtunA@mail.gmail.com>


Hi,

Rob Herring <robh+dt@kernel.org> writes:
>> Bjorn Andersson <bjorn.andersson@linaro.org> writes:
>> > On Wed 07 Jul 20:06 PDT 2021, Peter Chen wrote:
>> >
>> >> On 21-07-07 14:03:19, Bjorn Andersson wrote:
>> >> > On Tue 06 Jul 20:57 CDT 2021, Peter Chen wrote:
>> >> >
>> >> > Allow me to reorder your two questions:
>> >> >
>> >> > > And why using a notifier need to concern core's deferral probe?
>> >> >
>> >> > The problem at hand calls for the core for somehow invoking
>> >> > dwc3_qcom_vbus_overrride_enable() with a pointer to dwc3_qcom passed.
>> >> >
>> >> > This means that dwc3-qcom somehow needs to inform the dwc3-core about
>> >> > this (and stash the pointer). And this can't be done until dwc3-core
>> >> > actually exist, which it won't until dwc3_probe() has completed
>> >> > successfully (or in particular allocated struct dwc).
>> >>
>> >> Maybe you misunderstood the notifier I meant previous, my pointer was
>> >> calling glue layer API directly.
>> >>
>> >> Role switch is from dwc3-core, when it occurs, it means structure dwc3 has
>> >> allocated successfully, you could call glue layer notifier at function
>> >> dwc3_usb_role_switch_set directly.
>> >> Some references of my idea [1] [2]
>> >>
>> >> [1] Function ci_hdrc_msm_notify_event at ci_hdrc_msm_notify_event
>> >> [2] https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/usb/dwc3/core.c?h=lf-5.10.y#n205
>> >>
>> >
>> > Hi Peter, I took a proper look at this again, hoping to find a way to
>> > pass a callback pointer from dwc3-qcom to the dwc3 core, that can be
>> > called from __dwc3_set_mode() to inform the Qualcomm glue about mode
>> > changes.
>>
>> I would rather keep the strict separation between glue and core.
>
> On the surface that seems nice, but obviously there are issues with
> the approach. It's also not how pretty much every other instance of
> licensed IP blocks are structured.
>
> The specific need here seems to be multiple entities needing role
> switch notifications. Seems like that should be solved in a generic
> way.

right, solve it generically without breaking the module isolation ;-)

-- 
balbi