[PATCH 00/15] Initial MSM8953 & Fairphone 3 support

[PATCH 00/15] Initial MSM8953 & Fairphone 3 support

[Luca Weiss]

This series adds initial support for MSM8953 (and SDM632 which is based
on MSM8953) and the Fairphone 3 smartphone.

Only relatively basic functionality is supported like storage, volume
keys and USB.

There is currently close-to-mainline support for other components for
this SoC including GPU, WiFi and audio, this series adds only basic
support so that the other components can start getting upstreamed
easier.

Luca Weiss (10):
  dt-bindings: phy: qcom,qusb2: Document msm8953 compatible
  phy: qcom-qusb2: Add compatible for MSM8953
  dt-bindings: mfd: qcom,tcsr: Document msm8953 compatible
  mfd: qcom-spmi-pmic: Add pm8953 compatible
  dt-bindings: mmc: sdhci-msm: Add msm8953 compatible
  dt-bindings: thermal: tsens: Add msm8953 compatible
  dt-bindings: usb: qcom,dwc3: Add msm8953 compatible
  dt-bindings: pinctrl: qcom: msm8953: allow gpio-reserved-ranges
  dt-bindings: arm: qcom: Document sdm632 and fairphone,fp3 board
  arm64: dts: qcom: sdm632: Add device tree for Fairphone 3

Vladimir Lypak (5):
  rpmsg: smd: Drop unnecessary condition for channel creation
  arm64: dts: qcom: Add MSM8953 device tree
  arm64: dts: qcom: Add PM8953 PMIC
  arm64: dts: qcom: Add SDM632 device tree
  arm64: dts: qcom: Add MSM8953+PM8953 device tree

 .../devicetree/bindings/arm/qcom.yaml         |    6 +
 .../bindings/mfd/qcom,spmi-pmic.txt           |    1 +
 .../devicetree/bindings/mfd/qcom,tcsr.txt     |    1 +
 .../devicetree/bindings/mmc/sdhci-msm.txt     |    1 +
 .../bindings/phy/qcom,qusb2-phy.yaml          |    1 +
 .../pinctrl/qcom,msm8953-pinctrl.yaml         |    2 +
 .../bindings/thermal/qcom-tsens.yaml          |    1 +
 .../devicetree/bindings/usb/qcom,dwc3.yaml    |    1 +
 arch/arm64/boot/dts/qcom/Makefile             |    1 +
 arch/arm64/boot/dts/qcom/msm8953-pm8953.dtsi  |   50 +
 arch/arm64/boot/dts/qcom/msm8953.dtsi         | 1337 +++++++++++++++++
 arch/arm64/boot/dts/qcom/pm8953.dtsi          |   90 ++
 .../boot/dts/qcom/sdm632-fairphone-fp3.dts    |  189 +++
 arch/arm64/boot/dts/qcom/sdm632.dtsi          |  125 ++
 drivers/phy/qualcomm/phy-qcom-qusb2.c         |    3 +
 drivers/rpmsg/qcom_smd.c                      |    8 +-
 16 files changed, 1810 insertions(+), 7 deletions(-)
 create mode 100644 arch/arm64/boot/dts/qcom/msm8953-pm8953.dtsi
 create mode 100644 arch/arm64/boot/dts/qcom/msm8953.dtsi
 create mode 100644 arch/arm64/boot/dts/qcom/pm8953.dtsi
 create mode 100644 arch/arm64/boot/dts/qcom/sdm632-fairphone-fp3.dts
 create mode 100644 arch/arm64/boot/dts/qcom/sdm632.dtsi

-- 
2.34.1

[PATCH 09/15] rpmsg: smd: Drop unnecessary condition for channel creation

[Luca Weiss]

From: Vladimir Lypak <vladimir.lypak@gmail.com>

RPM Firmware on variety of newer SoCs such as MSM8917 (also likely
MSM8937, MSM8940, MSM8952), MSM8953 and on some MSM8916 devices) doesn't
initiate opening of the SMD channel if it was previously opened by
bootloader. This doesn't allow probing of smd-rpm driver on such devices
because there is a check that requires RPM this behaviour.

Signed-off-by: Vladimir Lypak <vladimir.lypak@gmail.com>
Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>
---
 drivers/rpmsg/qcom_smd.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/rpmsg/qcom_smd.c b/drivers/rpmsg/qcom_smd.c
index 8da1b5cb31b3..6a01ef932b01 100644
--- a/drivers/rpmsg/qcom_smd.c
+++ b/drivers/rpmsg/qcom_smd.c
@@ -1280,19 +1280,13 @@ static void qcom_channel_state_worker(struct work_struct *work)
 	unsigned long flags;
 
 	/*
-	 * Register a device for any closed channel where the remote processor
-	 * is showing interest in opening the channel.
+	 * Register a device for any closed channel.
 	 */
 	spin_lock_irqsave(&edge->channels_lock, flags);
 	list_for_each_entry(channel, &edge->channels, list) {
 		if (channel->state != SMD_CHANNEL_CLOSED)
 			continue;
 
-		remote_state = GET_RX_CHANNEL_INFO(channel, state);
-		if (remote_state != SMD_CHANNEL_OPENING &&
-		    remote_state != SMD_CHANNEL_OPENED)
-			continue;
-
 		if (channel->registered)
 			continue;
 
-- 
2.34.1

Re: [PATCH 09/15] rpmsg: smd: Drop unnecessary condition for channel creation

[Stephan Gerhold]

Hi,

+Cc Srinivas

On Wed, Jan 12, 2022 at 08:40:58PM +0100, Luca Weiss wrote:
> From: Vladimir Lypak <vladimir.lypak@gmail.com>
> 
> RPM Firmware on variety of newer SoCs such as MSM8917 (also likely
> MSM8937, MSM8940, MSM8952), MSM8953 and on some MSM8916 devices) doesn't
> initiate opening of the SMD channel if it was previously opened by
> bootloader. This doesn't allow probing of smd-rpm driver on such devices
> because there is a check that requires RPM this behaviour.
> 
> Signed-off-by: Vladimir Lypak <vladimir.lypak@gmail.com>
> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>

This is effectively a "Revert "Revert "rpmsg: smd: Create device for all
channels""":

https://lore.kernel.org/linux-arm-msm/20171212235857.10432-3-bjorn.andersson@linaro.org/
https://lore.kernel.org/linux-arm-msm/20180315181244.8859-1-bjorn.andersson@linaro.org/

Won't this cause the same regression reported by Srinivas again?

Thanks,
Stephan

> ---
>  drivers/rpmsg/qcom_smd.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/rpmsg/qcom_smd.c b/drivers/rpmsg/qcom_smd.c
> index 8da1b5cb31b3..6a01ef932b01 100644
> --- a/drivers/rpmsg/qcom_smd.c
> +++ b/drivers/rpmsg/qcom_smd.c
> @@ -1280,19 +1280,13 @@ static void qcom_channel_state_worker(struct work_struct *work)
>  	unsigned long flags;
>  
>  	/*
> -	 * Register a device for any closed channel where the remote processor
> -	 * is showing interest in opening the channel.
> +	 * Register a device for any closed channel.
>  	 */
>  	spin_lock_irqsave(&edge->channels_lock, flags);
>  	list_for_each_entry(channel, &edge->channels, list) {
>  		if (channel->state != SMD_CHANNEL_CLOSED)
>  			continue;
>  
> -		remote_state = GET_RX_CHANNEL_INFO(channel, state);
> -		if (remote_state != SMD_CHANNEL_OPENING &&
> -		    remote_state != SMD_CHANNEL_OPENED)
> -			continue;
> -
>  		if (channel->registered)
>  			continue;
>  
> -- 
> 2.34.1
> 

Re: [PATCH 09/15] rpmsg: smd: Drop unnecessary condition for channel creation

[Luca Weiss]

Hi Stephan,

On Mittwoch, 12. Jänner 2022 22:39:53 CET Stephan Gerhold wrote:
> Hi,
> 
> +Cc Srinivas
> 
> On Wed, Jan 12, 2022 at 08:40:58PM +0100, Luca Weiss wrote:
> > From: Vladimir Lypak <vladimir.lypak@gmail.com>
> > 
> > RPM Firmware on variety of newer SoCs such as MSM8917 (also likely
> > MSM8937, MSM8940, MSM8952), MSM8953 and on some MSM8916 devices) doesn't
> > initiate opening of the SMD channel if it was previously opened by
> > bootloader. This doesn't allow probing of smd-rpm driver on such devices
> > because there is a check that requires RPM this behaviour.
> > 
> > Signed-off-by: Vladimir Lypak <vladimir.lypak@gmail.com>
> > Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> > Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>
> 
> This is effectively a "Revert "Revert "rpmsg: smd: Create device for all
> channels""":
> 
> https://lore.kernel.org/linux-arm-msm/20171212235857.10432-3-bjorn.andersson
> @linaro.org/
> https://lore.kernel.org/linux-arm-msm/20180315181244.8859-1-bjorn.andersson
> @linaro.org/
> 
> Won't this cause the same regression reported by Srinivas again?
> 

Do you have any suggestion on another way to solve this? Without this commit 
the regulators just won't probe at all, I haven't looked very deep into it 
though given this patch solves it.

I guess worst case it'll become a devicetree property to enable this quirk?

Regards
Luca

> Thanks,
> Stephan
> 
> > ---
> > 
> >  drivers/rpmsg/qcom_smd.c | 8 +-------
> >  1 file changed, 1 insertion(+), 7 deletions(-)
> > 
> > diff --git a/drivers/rpmsg/qcom_smd.c b/drivers/rpmsg/qcom_smd.c
> > index 8da1b5cb31b3..6a01ef932b01 100644
> > --- a/drivers/rpmsg/qcom_smd.c
> > +++ b/drivers/rpmsg/qcom_smd.c
> > @@ -1280,19 +1280,13 @@ static void qcom_channel_state_worker(struct
> > work_struct *work)> 
> >  	unsigned long flags;
> >  	
> >  	/*
> > 
> > -	 * Register a device for any closed channel where the remote 
processor
> > -	 * is showing interest in opening the channel.
> > +	 * Register a device for any closed channel.
> > 
> >  	 */
> >  	
> >  	spin_lock_irqsave(&edge->channels_lock, flags);
> >  	list_for_each_entry(channel, &edge->channels, list) {
> >  	
> >  		if (channel->state != SMD_CHANNEL_CLOSED)
> >  		
> >  			continue;
> > 
> > -		remote_state = GET_RX_CHANNEL_INFO(channel, state);
> > -		if (remote_state != SMD_CHANNEL_OPENING &&
> > -		    remote_state != SMD_CHANNEL_OPENED)
> > -			continue;
> > -
> > 
> >  		if (channel->registered)
> >  		
> >  			continue;

Re: [PATCH 09/15] rpmsg: smd: Drop unnecessary condition for channel creation

[Stephan Gerhold]

On Sun, Jan 16, 2022 at 05:08:29PM +0100, Luca Weiss wrote:
> On Mittwoch, 12. Jänner 2022 22:39:53 CET Stephan Gerhold wrote:
> > On Wed, Jan 12, 2022 at 08:40:58PM +0100, Luca Weiss wrote:
> > > From: Vladimir Lypak <vladimir.lypak@gmail.com>
> > > 
> > > RPM Firmware on variety of newer SoCs such as MSM8917 (also likely
> > > MSM8937, MSM8940, MSM8952), MSM8953 and on some MSM8916 devices) doesn't
> > > initiate opening of the SMD channel if it was previously opened by
> > > bootloader. This doesn't allow probing of smd-rpm driver on such devices
> > > because there is a check that requires RPM this behaviour.
> > > 
> > > Signed-off-by: Vladimir Lypak <vladimir.lypak@gmail.com>
> > > Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> > > Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>
> > 
> > This is effectively a "Revert "Revert "rpmsg: smd: Create device for all
> > channels""":
> > 
> > https://lore.kernel.org/linux-arm-msm/20171212235857.10432-3-bjorn.andersson
> > @linaro.org/
> > https://lore.kernel.org/linux-arm-msm/20180315181244.8859-1-bjorn.andersson
> > @linaro.org/
> > 
> > Won't this cause the same regression reported by Srinivas again?
> > 
> 
> Do you have any suggestion on another way to solve this? Without this commit 
> the regulators just won't probe at all, I haven't looked very deep into it 
> though given this patch solves it.
> 
> I guess worst case it'll become a devicetree property to enable this quirk?
> 

My spontaneous suggestion would be to skip the check only for the
"rpm_requests" channel, e.g. something like

	if (remote_state != SMD_CHANNEL_OPENING &&
	    remote_state != SMD_CHANNEL_OPENED &&
	    strcmp(channel->name, "rpm_requests")
		continue;

This will avoid changing the behavior for anything but the RPM channel.
I don't think anything else is affected by the same problem (since the
bootloader or earlier firmware should not make use of any other channel).
Also, we definitely *always* want to open the channel to the RPM because
otherwise almost everything breaks.

Many solutions are possible though so at the end it is mostly up to
Bjorn to decide I think. :)

Thanks,
Stephan

Re: [PATCH 09/15] rpmsg: smd: Drop unnecessary condition for channel creation

[Bjorn Andersson]

On Sun 16 Jan 10:30 CST 2022, Stephan Gerhold wrote:

> On Sun, Jan 16, 2022 at 05:08:29PM +0100, Luca Weiss wrote:
> > On Mittwoch, 12. Jänner 2022 22:39:53 CET Stephan Gerhold wrote:
> > > On Wed, Jan 12, 2022 at 08:40:58PM +0100, Luca Weiss wrote:
> > > > From: Vladimir Lypak <vladimir.lypak@gmail.com>
> > > > 
> > > > RPM Firmware on variety of newer SoCs such as MSM8917 (also likely
> > > > MSM8937, MSM8940, MSM8952), MSM8953 and on some MSM8916 devices) doesn't
> > > > initiate opening of the SMD channel if it was previously opened by
> > > > bootloader. This doesn't allow probing of smd-rpm driver on such devices
> > > > because there is a check that requires RPM this behaviour.
> > > > 
> > > > Signed-off-by: Vladimir Lypak <vladimir.lypak@gmail.com>
> > > > Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> > > > Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>
> > > 
> > > This is effectively a "Revert "Revert "rpmsg: smd: Create device for all
> > > channels""":
> > > 
> > > https://lore.kernel.org/linux-arm-msm/20171212235857.10432-3-bjorn.andersson
> > > @linaro.org/
> > > https://lore.kernel.org/linux-arm-msm/20180315181244.8859-1-bjorn.andersson
> > > @linaro.org/
> > > 
> > > Won't this cause the same regression reported by Srinivas again?
> > > 
> > 
> > Do you have any suggestion on another way to solve this? Without this commit 
> > the regulators just won't probe at all, I haven't looked very deep into it 
> > though given this patch solves it.
> > 
> > I guess worst case it'll become a devicetree property to enable this quirk?
> > 
> 
> My spontaneous suggestion would be to skip the check only for the
> "rpm_requests" channel, e.g. something like
> 
> 	if (remote_state != SMD_CHANNEL_OPENING &&
> 	    remote_state != SMD_CHANNEL_OPENED &&
> 	    strcmp(channel->name, "rpm_requests")
> 		continue;
> 
> This will avoid changing the behavior for anything but the RPM channel.
> I don't think anything else is affected by the same problem (since the
> bootloader or earlier firmware should not make use of any other channel).
> Also, we definitely *always* want to open the channel to the RPM because
> otherwise almost everything breaks.
> 

Last time this came up I asked if someone could test if the RPM is stuck
in the state machine trying to close the channel and as such we could
kick it by making sure that we "ack" the closing of the channel and
hence it would come back up again.

But I don't remember seeing any outcome of this.

> Many solutions are possible though so at the end it is mostly up to
> Bjorn to decide I think. :)
> 

I would prefer to get an answer to above question, but if that doesn't
work (or look like crap) I'm willing to take your suggestion of skipping
the continue for the rpm_requests channel. Obviously with a comment
above describing why we're carrying that special case.

Regards,
Bjorn

Re: [PATCH 09/15] rpmsg: smd: Drop unnecessary condition for channel creation

[Bjorn Andersson]

On Wed 12 Jan 13:40 CST 2022, Luca Weiss wrote:

> From: Vladimir Lypak <vladimir.lypak@gmail.com>
> 
> RPM Firmware on variety of newer SoCs such as MSM8917 (also likely
> MSM8937, MSM8940, MSM8952), MSM8953 and on some MSM8916 devices) doesn't
> initiate opening of the SMD channel if it was previously opened by
> bootloader. This doesn't allow probing of smd-rpm driver on such devices
> because there is a check that requires RPM this behaviour.
> 
> Signed-off-by: Vladimir Lypak <vladimir.lypak@gmail.com>
> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>
> ---
>  drivers/rpmsg/qcom_smd.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/rpmsg/qcom_smd.c b/drivers/rpmsg/qcom_smd.c
> index 8da1b5cb31b3..6a01ef932b01 100644
> --- a/drivers/rpmsg/qcom_smd.c
> +++ b/drivers/rpmsg/qcom_smd.c
> @@ -1280,19 +1280,13 @@ static void qcom_channel_state_worker(struct work_struct *work)
>  	unsigned long flags;
>  
>  	/*
> -	 * Register a device for any closed channel where the remote processor
> -	 * is showing interest in opening the channel.
> +	 * Register a device for any closed channel.
>  	 */
>  	spin_lock_irqsave(&edge->channels_lock, flags);
>  	list_for_each_entry(channel, &edge->channels, list) {
>  		if (channel->state != SMD_CHANNEL_CLOSED)
>  			continue;
>  
> -		remote_state = GET_RX_CHANNEL_INFO(channel, state);
> -		if (remote_state != SMD_CHANNEL_OPENING &&
> -		    remote_state != SMD_CHANNEL_OPENED)
> -			continue;

The second time you boot the modem (e.g. after a firmware crash), we
will find a whole bunch of channels here and attempt to open them in
order, but the modem will refuse to open most of them until the IPCRTR
channel has been opened and we have done the rmtfs dance - at which time
we have timed out opening a bunch of channels and things are in a broken
state.

As such, this has been proven to not work :(

Regards,
Bjorn

> -
>  		if (channel->registered)
>  			continue;
>  
> -- 
> 2.34.1
> 

Re: [PATCH 09/15] rpmsg: smd: Drop unnecessary condition for channel creation

[Luca Weiss]

Hi Bjorn,

On Montag, 31. Jänner 2022 23:32:42 CET Bjorn Andersson wrote:
> On Sun 16 Jan 10:30 CST 2022, Stephan Gerhold wrote:
> > On Sun, Jan 16, 2022 at 05:08:29PM +0100, Luca Weiss wrote:
> > > On Mittwoch, 12. Jänner 2022 22:39:53 CET Stephan Gerhold wrote:
> > > > On Wed, Jan 12, 2022 at 08:40:58PM +0100, Luca Weiss wrote:
> > > > > From: Vladimir Lypak <vladimir.lypak@gmail.com>
> > > > > 
> > > > > RPM Firmware on variety of newer SoCs such as MSM8917 (also likely
> > > > > MSM8937, MSM8940, MSM8952), MSM8953 and on some MSM8916 devices)
> > > > > doesn't
> > > > > initiate opening of the SMD channel if it was previously opened by
> > > > > bootloader. This doesn't allow probing of smd-rpm driver on such
> > > > > devices
> > > > > because there is a check that requires RPM this behaviour.
> > > > > 
> > > > > Signed-off-by: Vladimir Lypak <vladimir.lypak@gmail.com>
> > > > > Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> > > > > Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>
> > > > 
> > > > This is effectively a "Revert "Revert "rpmsg: smd: Create device for
> > > > all
> > > > channels""":
> > > > 
> > > > https://lore.kernel.org/linux-arm-msm/20171212235857.10432-3-bjorn.and
> > > > ersson @linaro.org/
> > > > https://lore.kernel.org/linux-arm-msm/20180315181244.8859-1-bjorn.ande
> > > > rsson
> > > > @linaro.org/
> > > > 
> > > > Won't this cause the same regression reported by Srinivas again?
> > > 
> > > Do you have any suggestion on another way to solve this? Without this
> > > commit the regulators just won't probe at all, I haven't looked very
> > > deep into it though given this patch solves it.
> > > 
> > > I guess worst case it'll become a devicetree property to enable this
> > > quirk?
> > 
> > My spontaneous suggestion would be to skip the check only for the
> > "rpm_requests" channel, e.g. something like
> > 
> > 	if (remote_state != SMD_CHANNEL_OPENING &&
> > 	
> > 	    remote_state != SMD_CHANNEL_OPENED &&
> > 	    strcmp(channel->name, "rpm_requests")
> > 		
> > 		continue;
> > 
> > This will avoid changing the behavior for anything but the RPM channel.
> > I don't think anything else is affected by the same problem (since the
> > bootloader or earlier firmware should not make use of any other channel).
> > Also, we definitely *always* want to open the channel to the RPM because
> > otherwise almost everything breaks.
> 
> Last time this came up I asked if someone could test if the RPM is stuck
> in the state machine trying to close the channel and as such we could
> kick it by making sure that we "ack" the closing of the channel and
> hence it would come back up again.
> 
> But I don't remember seeing any outcome of this.

Do you have a link to this or should I go digging in the archives?

Regards
Luca

> 
> > Many solutions are possible though so at the end it is mostly up to
> > Bjorn to decide I think. :)
> 
> I would prefer to get an answer to above question, but if that doesn't
> work (or look like crap) I'm willing to take your suggestion of skipping
> the continue for the rpm_requests channel. Obviously with a comment
> above describing why we're carrying that special case.
> 
> Regards,
> Bjorn

Re: [PATCH 09/15] rpmsg: smd: Drop unnecessary condition for channel creation

[Luca Weiss]

Hi Bjorn,

On Sonntag, 6. Februar 2022 21:17:22 CET Luca Weiss wrote:
> Hi Bjorn,
> 
> On Montag, 31. Jänner 2022 23:32:42 CET Bjorn Andersson wrote:
> > On Sun 16 Jan 10:30 CST 2022, Stephan Gerhold wrote:
> > > On Sun, Jan 16, 2022 at 05:08:29PM +0100, Luca Weiss wrote:
> > > > On Mittwoch, 12. Jänner 2022 22:39:53 CET Stephan Gerhold wrote:
> > > > > On Wed, Jan 12, 2022 at 08:40:58PM +0100, Luca Weiss wrote:
> > > > > > From: Vladimir Lypak <vladimir.lypak@gmail.com>
> > > > > > 
> > > > > > RPM Firmware on variety of newer SoCs such as MSM8917 (also likely
> > > > > > MSM8937, MSM8940, MSM8952), MSM8953 and on some MSM8916 devices)
> > > > > > doesn't
> > > > > > initiate opening of the SMD channel if it was previously opened by
> > > > > > bootloader. This doesn't allow probing of smd-rpm driver on such
> > > > > > devices
> > > > > > because there is a check that requires RPM this behaviour.
> > > > > > 
> > > > > > Signed-off-by: Vladimir Lypak <vladimir.lypak@gmail.com>
> > > > > > Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> > > > > > Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>
> > > > > 
> > > > > This is effectively a "Revert "Revert "rpmsg: smd: Create device for
> > > > > all
> > > > > channels""":
> > > > > 
> > > > > https://lore.kernel.org/linux-arm-msm/20171212235857.10432-3-bjorn.a
> > > > > nd
> > > > > ersson @linaro.org/
> > > > > https://lore.kernel.org/linux-arm-msm/20180315181244.8859-1-bjorn.an
> > > > > de
> > > > > rsson
> > > > > @linaro.org/
> > > > > 
> > > > > Won't this cause the same regression reported by Srinivas again?
> > > > 
> > > > Do you have any suggestion on another way to solve this? Without this
> > > > commit the regulators just won't probe at all, I haven't looked very
> > > > deep into it though given this patch solves it.
> > > > 
> > > > I guess worst case it'll become a devicetree property to enable this
> > > > quirk?
> > > 
> > > My spontaneous suggestion would be to skip the check only for the
> > > "rpm_requests" channel, e.g. something like
> > > 
> > > 	if (remote_state != SMD_CHANNEL_OPENING &&
> > > 	
> > > 	    remote_state != SMD_CHANNEL_OPENED &&
> > > 	    strcmp(channel->name, "rpm_requests")
> > > 		
> > > 		continue;
> > > 
> > > This will avoid changing the behavior for anything but the RPM channel.
> > > I don't think anything else is affected by the same problem (since the
> > > bootloader or earlier firmware should not make use of any other
> > > channel).
> > > Also, we definitely *always* want to open the channel to the RPM because
> > > otherwise almost everything breaks.
> > 
> > Last time this came up I asked if someone could test if the RPM is stuck
> > in the state machine trying to close the channel and as such we could
> > kick it by making sure that we "ack" the closing of the channel and
> > hence it would come back up again.
> > 
> > But I don't remember seeing any outcome of this.
> 
> Do you have a link to this or should I go digging in the archives?

Replying to myself, I went searching but couldn't find anything. If you have 
some PoC code I'd be happy to try but as I'm not familiar with rpm/smd at all 
I'd have to read myself into it first.

If Stephans suggestion with the strcmp(channel->name, "rpm_requests") is ok 
then I'd test this and use that in v2. I'd personally rather not spend too 
much time on this issue right now as it's blocking msm8953 completely (no 
regulators = no nothing),

Regards
Luca

> 
> Regards
> Luca
> 
> > > Many solutions are possible though so at the end it is mostly up to
> > > Bjorn to decide I think. :)
> > 
> > I would prefer to get an answer to above question, but if that doesn't
> > work (or look like crap) I'm willing to take your suggestion of skipping
> > the continue for the rpm_requests channel. Obviously with a comment
> > above describing why we're carrying that special case.
> > 
> > Regards,
> > Bjorn

Re: [PATCH 09/15] rpmsg: smd: Drop unnecessary condition for channel creation

[Bjorn Andersson]

On Sun 13 Feb 14:51 CST 2022, Luca Weiss wrote:

> Hi Bjorn,
> 
> On Sonntag, 6. Februar 2022 21:17:22 CET Luca Weiss wrote:
> > Hi Bjorn,
> > 
> > On Montag, 31. Jänner 2022 23:32:42 CET Bjorn Andersson wrote:
> > > On Sun 16 Jan 10:30 CST 2022, Stephan Gerhold wrote:
> > > > On Sun, Jan 16, 2022 at 05:08:29PM +0100, Luca Weiss wrote:
> > > > > On Mittwoch, 12. Jänner 2022 22:39:53 CET Stephan Gerhold wrote:
> > > > > > On Wed, Jan 12, 2022 at 08:40:58PM +0100, Luca Weiss wrote:
> > > > > > > From: Vladimir Lypak <vladimir.lypak@gmail.com>
> > > > > > > 
> > > > > > > RPM Firmware on variety of newer SoCs such as MSM8917 (also likely
> > > > > > > MSM8937, MSM8940, MSM8952), MSM8953 and on some MSM8916 devices)
> > > > > > > doesn't
> > > > > > > initiate opening of the SMD channel if it was previously opened by
> > > > > > > bootloader. This doesn't allow probing of smd-rpm driver on such
> > > > > > > devices
> > > > > > > because there is a check that requires RPM this behaviour.
> > > > > > > 
> > > > > > > Signed-off-by: Vladimir Lypak <vladimir.lypak@gmail.com>
> > > > > > > Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> > > > > > > Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>
> > > > > > 
> > > > > > This is effectively a "Revert "Revert "rpmsg: smd: Create device for
> > > > > > all
> > > > > > channels""":
> > > > > > 
> > > > > > https://lore.kernel.org/linux-arm-msm/20171212235857.10432-3-bjorn.a
> > > > > > nd
> > > > > > ersson @linaro.org/
> > > > > > https://lore.kernel.org/linux-arm-msm/20180315181244.8859-1-bjorn.an
> > > > > > de
> > > > > > rsson
> > > > > > @linaro.org/
> > > > > > 
> > > > > > Won't this cause the same regression reported by Srinivas again?
> > > > > 
> > > > > Do you have any suggestion on another way to solve this? Without this
> > > > > commit the regulators just won't probe at all, I haven't looked very
> > > > > deep into it though given this patch solves it.
> > > > > 
> > > > > I guess worst case it'll become a devicetree property to enable this
> > > > > quirk?
> > > > 
> > > > My spontaneous suggestion would be to skip the check only for the
> > > > "rpm_requests" channel, e.g. something like
> > > > 
> > > > 	if (remote_state != SMD_CHANNEL_OPENING &&
> > > > 	
> > > > 	    remote_state != SMD_CHANNEL_OPENED &&
> > > > 	    strcmp(channel->name, "rpm_requests")
> > > > 		
> > > > 		continue;
> > > > 
> > > > This will avoid changing the behavior for anything but the RPM channel.
> > > > I don't think anything else is affected by the same problem (since the
> > > > bootloader or earlier firmware should not make use of any other
> > > > channel).
> > > > Also, we definitely *always* want to open the channel to the RPM because
> > > > otherwise almost everything breaks.
> > > 
> > > Last time this came up I asked if someone could test if the RPM is stuck
> > > in the state machine trying to close the channel and as such we could
> > > kick it by making sure that we "ack" the closing of the channel and
> > > hence it would come back up again.
> > > 
> > > But I don't remember seeing any outcome of this.
> > 
> > Do you have a link to this or should I go digging in the archives?
> 
> Replying to myself, I went searching but couldn't find anything. If you have 
> some PoC code I'd be happy to try but as I'm not familiar with rpm/smd at all 
> I'd have to read myself into it first.
> 

A quick search didn't turn anything up on my side either.

And while I had suggestions of what could be tried, I don't have any
devices myself that manifest this problem, so I haven't been able to
debug it.

> If Stephans suggestion with the strcmp(channel->name, "rpm_requests") is ok 
> then I'd test this and use that in v2. I'd personally rather not spend too 
> much time on this issue right now as it's blocking msm8953 completely (no 
> regulators = no nothing),
> 

It's been a long time since this problem was initially reported, so I
rather see us land the strcmp() hack to unblock you and others. Then
someone who knows SMD can take a proper look at this.

Regards,
Bjorn

> Regards
> Luca
> 
> > 
> > Regards
> > Luca
> > 
> > > > Many solutions are possible though so at the end it is mostly up to
> > > > Bjorn to decide I think. :)
> > > 
> > > I would prefer to get an answer to above question, but if that doesn't
> > > work (or look like crap) I'm willing to take your suggestion of skipping
> > > the continue for the rpm_requests channel. Obviously with a comment
> > > above describing why we're carrying that special case.
> > > 
> > > Regards,
> > > Bjorn
> 
> 
> 
>