Re: (subset) [PATCH v2 0/7] Initial support for the Fairphone 5 smartphone

2023-09-20 Thread Bjorn Andersson


On Tue, 19 Sep 2023 14:45:54 +0200, Luca Weiss wrote:
> Add support to boot up mainline kernel on the QCM6490-based Fairphone 5
> smartphone.
> 
> These patches only cover a part of the functionality brought up on
> mainline so far, with the rest needing larger dts and driver changes or
> depend on patches that are not yet merged. I will work on sending those
> once these base patches here have settled.
> 
> [...]

Applied, thanks!

[1/7] arm64: dts: qcom: sc7280: Mark some nodes as 'reserved'
  commit: 6da24ba932082bae110feb917a64bb54637fa7c0
[3/7] arm64: dts: qcom: pm7250b: make SID configurable
  commit: 8e2d56f64572e0432c355093a7601bde29677490
[4/7] arm64: dts: qcom: pm8350c: Add flash led node
  commit: bfd4412a023b2a3a2f858f2ffc13705aaeef5737
[6/7] dt-bindings: arm: qcom: Add QCM6490 Fairphone 5
  commit: 4b1a16d776b474345b12f834de1fd42bca226d90
[7/7] arm64: dts: qcom: qcm6490: Add device-tree for Fairphone 5
  commit: eee9602ad6498eee9ddab1b7eb6aede288f0b934

Best regards,
-- 
Bjorn Andersson 


Re: (subset) [PATCH v2 00/14] Clean up RPM bus clocks remnants

2023-09-20 Thread Bjorn Andersson


On Tue, 12 Sep 2023 15:31:38 +0200, Konrad Dybcio wrote:
> After the recent cleanups ([1], [2]) some in-tree abusers that directly
> accessed the RPM bus clocks, effectively circumventing and working
> against the efforts of the interconnect framework, were found.
> 
> Patches 1-5 drop deprecated references and the rest attempt to stop
> direct bus clock abuses.
> 
> [...]

Applied, thanks!

[08/14] dt-bindings: remoteproc: qcom,adsp: Remove AGGRE2 clock
commit: c4c5b47958529bc1de10260df0c583710853b516
[09/14] dt-bindings: remoteproc: qcom,msm8996-mss-pil: Remove PNoC clock
commit: e7781901449cbcff129d80a5d9021e9e96084ec4
[10/14] remoteproc: qcom: q6v5-mss: Remove PNoC clock from 8996 MSS
commit: e1592981c51bac38ea2041b642777b3ba30606a8

Best regards,
-- 
Bjorn Andersson 


Re: (subset) [PATCH v3 0/7] MSM8976 PLL,RPMPD and DTS changes

2023-09-20 Thread Bjorn Andersson


On Sat, 12 Aug 2023 13:24:43 +0200, Adam Skladowski wrote:
> This patch series fixes introduce support for msm8976 pll,
> also brings some adjustments and fixes domains setup and few dts nitpicks.
> 
> Changes since v1
> 
> 1. Fixed few styling issues
> 2. Changed compatibles for plls
> 3. Added fixes: tag to first patch
> 
> [...]

Applied, thanks!

[6/7] arm64: dts: qcom: msm8976: Split lpass region
  commit: 31c133b4a07e3db456a7e661c96653cd65a25bc6
[7/7] arm64: dts: qcom: msm8976: Fix ipc bit shifts
  commit: 684277525c70f329300cc687e27248e405a4ff9e

Best regards,
-- 
Bjorn Andersson 


Re: (subset) [PATCH v3 0/4] sc7180: Add ADSP

2023-09-20 Thread Bjorn Andersson


On Thu, 07 Sep 2023 15:02:33 +0500, Nikita Travkin wrote:
> sc7180 has an ADSP remoteproc that can be used to control the sound
> hardware. This remoteproc has to be used on those devices that use
> Qualcomm firmware and thus are locked out of driving the lpass directly.
> 
> Introducing the ADSP would allow multiple WoA laptops such as Aspire 1
> to provide sound. It's also useful for the sm7125 devices that are to be
> included to the kernel [1]
> 
> [...]

Applied, thanks!

[3/4] arm64: dts: qcom: sc7180: Add tertiary mi2s pinctrl
  commit: 828298a9efb237b76fa667bb74a6450d1df3eeed
[4/4] arm64: dts: qcom: sc7180: Add ADSP
  commit: a3d5fb3b084c0c67f9918a866b92cbe09b9e1d77

Best regards,
-- 
Bjorn Andersson 


Re: [PATCH] clk: qcom: mmcc-msm8974: remove ocmemcx_ahb_clk

2023-09-20 Thread Bjorn Andersson


On Sat, 02 Sep 2023 19:34:23 +0200, Luca Weiss wrote:
> According to a commit in the 3.4 vendor kernel sources[0] the
> ocmemcx_ahb_clk clock "is controlled by RPM and should not be touched by
> APPS.".
> 
> [0] 
> https://git.codelinaro.org/clo/la/kernel/msm/-/commit/37df5f2d91b4d5768b37fcaacaeea958dd683ebc
> 
> And indeed, when using MDSS+GPU+OCMEM on MSM8226 and not using
> clk_ignore_unused, when Linux tries to disable the clock the device
> crashes and reboots.
> 
> [...]

Applied, thanks!

[1/1] clk: qcom: mmcc-msm8974: remove ocmemcx_ahb_clk
  commit: 471e2875f8904539985e62220afd6c88e70779fa

Best regards,
-- 
Bjorn Andersson 


Re: [PATCH v5 0/3] SM6375 remoteprocs

2023-09-20 Thread Bjorn Andersson


On Thu, 27 Jul 2023 19:33:20 +0200, Konrad Dybcio wrote:
> Resending as the previous revision was mostly ignored on the rproc side.
> 
> Changes since v3:
> - Pick up krzk's rb on bindings
> - Drop patch 4 (applied)
> Link to v3: 
> https://lore.kernel.org/linux-arm-msm/20230109135647.339224-1-konrad.dyb...@linaro.org/
> 
> [...]

Applied, thanks!

[1/3] dt-bindings: remoteproc: qcom,sm6375-pas: Document remoteprocs
  commit: 6d3211e015b0f236986b16c042f71cce8d36d727
[2/3] remoteproc: qcom: pas: Add SM6375 ADSP & CDSP
  commit: a6df21cf0c93cab57059e2592c7c99b424965374
[3/3] remoteproc: qcom: pas: Add SM6375 MPSS
  commit: 93f875645c9da9c788224964499e68fa9664e80f

Best regards,
-- 
Bjorn Andersson 


Re: (subset) [PATCH v2 0/2] Fix tcsr_mutex register for IPQ6018

2023-09-20 Thread Bjorn Andersson


On Tue, 05 Sep 2023 15:25:33 +0530, Vignesh Viswanathan wrote:
> IPQ6018 has 32 tcsr_mutex hwlock registers of 0x1000 size each.
> The compatible string qcom,ipq6018-tcsr-mutex is mapped to
> of_msm8226_tcsr_mutex which has 32 locks configured with stride of 0x80
> and doesn't match the HW present in IPQ6018.
> 
> This series fixes the following:
>  1. Fix the tcsr_mutex register size to 0x2 in IPQ6018 DTSI.
>  2. Remove IPQ6018 specific compatible in hwspinlock driver so that it
> falls back to pick of_tcsr_mutex data.
> 
> [...]

Applied, thanks!

[2/2] hwspinlock: qcom: Remove IPQ6018 SOC specific compatible
  commit: 823313068617bf2414c6067504b4e2ce5768e601

Best regards,
-- 
Bjorn Andersson 


Re: [PATCH v2] dt-bindings: remoteproc: pru: Add Interrupt property

2023-09-20 Thread Bjorn Andersson


On Mon, 14 Aug 2023 15:21:41 +0530, MD Danish Anwar wrote:
> Add interrupts and interrupt-names protperties for PRU and RTU cores.
> 
> 

Applied, thanks!

[1/1] dt-bindings: remoteproc: pru: Add Interrupt property
  commit: d93f191b95bec3c913978eb18c6297e797915993

Best regards,
-- 
Bjorn Andersson 


Re: (subset) [PATCH v2 0/2] Fix tcsr_mutex register for IPQ6018

2023-09-19 Thread Bjorn Andersson


On Tue, 05 Sep 2023 15:25:33 +0530, Vignesh Viswanathan wrote:
> IPQ6018 has 32 tcsr_mutex hwlock registers of 0x1000 size each.
> The compatible string qcom,ipq6018-tcsr-mutex is mapped to
> of_msm8226_tcsr_mutex which has 32 locks configured with stride of 0x80
> and doesn't match the HW present in IPQ6018.
> 
> This series fixes the following:
>  1. Fix the tcsr_mutex register size to 0x2 in IPQ6018 DTSI.
>  2. Remove IPQ6018 specific compatible in hwspinlock driver so that it
> falls back to pick of_tcsr_mutex data.
> 
> [...]

Applied, thanks!

[1/2] arm64: dts: qcom: ipq6018: Fix tcsr_mutex register size
  commit: 72fc3d58b87b0d622039c6299b89024fbb7b420f

Best regards,
-- 
Bjorn Andersson 


Re: [PATCH] arm64: dts: qcom: Use QCOM_SCM_VMID defines for qcom,vmid

2023-09-19 Thread Bjorn Andersson


On Fri, 18 Aug 2023 10:06:09 +0200, Luca Weiss wrote:
> Since we have those defines available in a header, let's use them
> everywhere where qcom,vmid property is used.
> 
> 

Applied, thanks!

[1/1] arm64: dts: qcom: Use QCOM_SCM_VMID defines for qcom,vmid
  commit: 018c949b32df9f17f52bf0e70f976719811db233

Best regards,
-- 
Bjorn Andersson 


Re: (subset) [PATCH v4 0/7] Add initial support for SM7125 and Xiaomi SM7125 platform

2023-09-19 Thread Bjorn Andersson


On Sun, 23 Jul 2023 21:05:01 +0200, David Wronek wrote:
> This series introduces support for the Qualcomm SM7125 SoC and the
> Xiaomi SM7125 platform.
> 
> 

Applied, thanks!

[3/7] dt-bindings: arm: qcom: Document SM7125 and xiaomi,joyeuse board
  commit: 9b4adf37fdc0ca8cd1d14b4160e2f04b63df98e6
[5/7] arm64: dts: qcom: pm6150: Add resin and rtc nodes
  commit: ec053ec90c245a4efc8dda87d9207de0adf0040e
[6/7] arm64: dts: qcom: Add SM7125 device tree
  commit: 72fbf05149bd451e7222c2ed1e3823972f19df9c
[7/7] arm64: dts: qcom: Add support for the Xiaomi SM7125 platform
  commit: 7d65d4b7d70fb9560ce9baaf4219fb24646bd578

Best regards,
-- 
Bjorn Andersson 


Re: [PATCH v4 0/2] arm64: dts: qcom: msm8939-longcheer-l9100: Add initial dts

2023-09-19 Thread Bjorn Andersson


On Sun, 27 Aug 2023 10:47:58 +0200, André Apitzsch wrote:
> This dts adds support for BQ Aquaris M5 (Longcheer L9100) released in
> 2015.
> 
> Add a device tree with initial support for:
> 
> - GPIO keys
> - Hall sensor
> - SDHCI
> - WCNSS (BT/WIFI)
> - Accelerometer/Magnetometer
> - Vibrator
> - Touchscreen
> - Front flash
> 
> [...]

Applied, thanks!

[1/2] dt-bindings: arm: qcom: Add BQ Aquaris M5
  commit: 9905205541d2020e45da5ffe9787b4a2e53cc199
[2/2] arm64: dts: qcom: msm8939-longcheer-l9100: Add initial device tree
  commit: 27da4fd325c371e1ddbb4fc46629e2caf8f73f07

Best regards,
-- 
Bjorn Andersson 


Re: [PATCH v5 0/4] Add initial support for SM7125 and Xiaomi SM7125 platform

2023-09-19 Thread Bjorn Andersson


On Thu, 24 Aug 2023 11:15:03 +0200, David Wronek wrote:
> This series introduces support for the Qualcomm SM7125 SoC and the
> Xiaomi SM7125 platform.
> 
> 

Applied, thanks!

[1/4] dt-bindings: arm: qcom: Document SM7125 and xiaomi,joyeuse board
  commit: 9b4adf37fdc0ca8cd1d14b4160e2f04b63df98e6
[2/4] arm64: dts: qcom: pm6150: Add resin and rtc nodes
  commit: ec053ec90c245a4efc8dda87d9207de0adf0040e
[3/4] arm64: dts: qcom: Add SM7125 device tree
  commit: 72fbf05149bd451e7222c2ed1e3823972f19df9c
[4/4] arm64: dts: qcom: Add support for the Xiaomi SM7125 platform
  commit: 7d65d4b7d70fb9560ce9baaf4219fb24646bd578

Best regards,
-- 
Bjorn Andersson 


Re: [PATCH v3] arm64: dts: qcom: sdm630: Add support for modem remoteproc

2023-09-19 Thread Bjorn Andersson


On Wed, 19 Jul 2023 12:34:58 +0300, Alexey Minnekhanov wrote:
> Modem subsystem in SDM630/660 is similar to MSM8998 and
> device tree node for it is based on the one from msm8998.dtsi.
> 
> 

Applied, thanks!

[1/1] arm64: dts: qcom: sdm630: Add support for modem remoteproc
  commit: 09f1642eca6eb6d25a630214098350dc02917954

Best regards,
-- 
Bjorn Andersson 


Re: (subset) [PATCH 00/11] Initial support for the Fairphone 5 smartphone

2023-09-14 Thread Bjorn Andersson


On Wed, 30 Aug 2023 11:58:25 +0200, Luca Weiss wrote:
> Add support to boot up mainline kernel on the QCM6490-based Fairphone 5
> smartphone.
> 
> These patches only cover a part of the functionality brought up on
> mainline so far, with the rest needing larger dts and driver changes or
> depend on patches that are not yet merged. I will work on sending those
> once these base patches here have settled.
> 
> [...]

Applied, thanks!

[07/11] dt-bindings: arm: qcom,ids: Add SoC ID for QCM6490
commit: ccfb4d8b606302d857a03ea29039e21029311335
[08/11] soc: qcom: socinfo: Add SoC ID for QCM6490
commit: 59872d59d164ec67f295d6f96fe818b92973ee40

Best regards,
-- 
Bjorn Andersson 


Re: (subset) [PATCH v3 0/7] MSM8976 PLL,RPMPD and DTS changes

2023-09-14 Thread Bjorn Andersson


On Sat, 12 Aug 2023 13:24:43 +0200, Adam Skladowski wrote:
> This patch series fixes introduce support for msm8976 pll,
> also brings some adjustments and fixes domains setup and few dts nitpicks.
> 
> Changes since v1
> 
> 1. Fixed few styling issues
> 2. Changed compatibles for plls
> 3. Added fixes: tag to first patch
> 
> [...]

Applied, thanks!

[2/7] clk: qcom: clk-hfpll: Configure l_val in init when required
  commit: 500a4609eef46d49a260173b66cabb20bd5159ad
[3/7] clk: qcom: hfpll: Allow matching pdata
  commit: 34e000c0963e55f24be2254fa645f8dd8257a9e0
[4/7] dt-bindings: clock: qcom,hfpll: Document MSM8976 compatibles
  commit: de37ca2dc98607e74522d8f243aa7feac74577c5
[5/7] clk: qcom: hfpll: Add MSM8976 PLL data
  commit: 1fa2d1a887c763246662a88e203d69b36052770c

Best regards,
-- 
Bjorn Andersson 


Re: [PATCH v3 1/7] drivers: genpd: qcom: rpmpd: Fix MSM8976 power domains setup

2023-09-13 Thread Bjorn Andersson
On Sat, Aug 12, 2023 at 01:24:44PM +0200, Adam Skladowski wrote:

Please drop the "drivers:" prefix in $subject, and resubmit this with
patch (alone should be fine) with the new maintainer, and appropriate
mailing list, included.

Thanks,
Bjorn

> Downstream kernel parses resource names based on pm8950-rpm-regulator.dtsi
> in such file qcom,resource-name takes three values: smpa,ldoa and clk0.
> First appearance of RWSC/RWSM point to msm-4.4 kernel
> which is way newer than what this platform was shipped with (msm-3.10).
> For the max_state downstream code limit value to TURBO inside dts
> with only one turbo_high being placed in msm-thermal bindings.
> One of effects of requesting TURBO_HIGH vote is rebooting of device
> which happens during voting inside WCNSS/IRIS,
> this behavior was observed on LeEco S2 smartphone.
> Fix regulator setup and drop unused resources.
> 
> Fixes: b1d522443b4b ("soc: qcom: rpmpd: Add rpm power domains for msm8976")
> Signed-off-by: Adam Skladowski 
> Reviewed-by: Dmitry Baryshkov 
> ---
>  drivers/genpd/qcom/rpmpd.c | 27 ++-
>  1 file changed, 10 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/genpd/qcom/rpmpd.c b/drivers/genpd/qcom/rpmpd.c
> index 3135dd1dafe0..59caa4e7b99a 100644
> --- a/drivers/genpd/qcom/rpmpd.c
> +++ b/drivers/genpd/qcom/rpmpd.c
> @@ -166,13 +166,6 @@ static struct rpmpd cx_rwcx0_vfl = {
>   .key = KEY_FLOOR_LEVEL,
>  };
>  
> -static struct rpmpd cx_rwsc2_vfl = {
> - .pd = { .name = "cx_vfl", },
> - .res_type = RPMPD_RWSC,
> - .res_id = 2,
> - .key = KEY_FLOOR_LEVEL,
> -};
> -
>  static struct rpmpd cx_s1a_vfc = {
>   .pd = { .name = "cx_vfc", },
>   .res_type = RPMPD_SMPA,
> @@ -329,6 +322,13 @@ static struct rpmpd mx_s6a_lvl_ao = {
>   .key = KEY_LEVEL,
>  };
>  
> +static struct rpmpd mx_s6a_vfl = {
> + .pd = { .name = "mx_vfl", },
> + .res_type = RPMPD_SMPA,
> + .res_id = 6,
> + .key = KEY_FLOOR_LEVEL,
> +};
> +
>  static struct rpmpd mx_s7a_lvl_ao;
>  static struct rpmpd mx_s7a_lvl = {
>   .pd = { .name = "mx", },
> @@ -361,13 +361,6 @@ static struct rpmpd mx_rwmx0_vfl = {
>   .key = KEY_FLOOR_LEVEL,
>  };
>  
> -static struct rpmpd mx_rwsm6_vfl = {
> - .pd = { .name = "mx_vfl", },
> - .res_type = RPMPD_RWSM,
> - .res_id = 6,
> - .key = KEY_FLOOR_LEVEL,
> -};
> -
>  /* MD */
>  static struct rpmpd md_s1a_corner_ao;
>  static struct rpmpd md_s1a_corner = {
> @@ -591,16 +584,16 @@ static const struct rpmpd_desc msm8953_desc = {
>  static struct rpmpd *msm8976_rpmpds[] = {
>   [MSM8976_VDDCX] =   _s2a_lvl,
>   [MSM8976_VDDCX_AO] =_s2a_lvl_ao,
> - [MSM8976_VDDCX_VFL] =   _rwsc2_vfl,
> + [MSM8976_VDDCX_VFL] =   _s2a_vfl,
>   [MSM8976_VDDMX] =   _s6a_lvl,
>   [MSM8976_VDDMX_AO] =_s6a_lvl_ao,
> - [MSM8976_VDDMX_VFL] =   _rwsm6_vfl,
> + [MSM8976_VDDMX_VFL] =   _s6a_vfl,
>  };
>  
>  static const struct rpmpd_desc msm8976_desc = {
>   .rpmpds = msm8976_rpmpds,
>   .num_pds = ARRAY_SIZE(msm8976_rpmpds),
> - .max_state = RPM_SMD_LEVEL_TURBO_HIGH,
> + .max_state = RPM_SMD_LEVEL_TURBO,
>  };
>  
>  static struct rpmpd *msm8994_rpmpds[] = {
> -- 
> 2.41.0
> 


Re: [PATCH v4 5/7] cpufreq: qcom-hw: Implement CPRh aware OSM programming

2021-04-19 Thread Bjorn Andersson
On Mon 19 Apr 15:59 CDT 2021, AngeloGioacchino Del Regno wrote:

> Il 19/04/21 20:52, Bjorn Andersson ha scritto:
> > On Tue 19 Jan 11:45 CST 2021, AngeloGioacchino Del Regno wrote:
[..]
> > > +static int qcom_cpufreq_hw_acd_init(struct device *cpu_dev,
> > > + struct cpufreq_policy *policy,
> > > + int index)
> > > +{
[..]
> > > + acd_resname = kasprintf(GFP_KERNEL, "osm-acd%d", index);
> > 
> > How about just sprintf() into a 10 byte array on the stack?
> > 
> 
> My motto, apart the clearly possible chance to get 1000 clusters in the
> future (lol), is to free the (very little) memory as soon as I'm done with
> it.
> 
> Was I too much paranoid there again? :)))
> 

Feel free to waste a couple of extra bytes in that array then ;)

[..]
> > >   static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)
[..]
> > > + /*
> > > +  * If the power domain device is not registered yet, then
> > > +  * defer probing this driver until that is available.
> > > +  */
> > > + pd_dev = of_find_device_by_node(pd_node);
> > > + if (!pd_dev || !pd_dev->dev.driver ||
> > > + !device_is_bound(_dev->dev))
> > > + return -EPROBE_DEFER;
> > 
> > I wonder if there's a more appropriate way to probe defer on resources
> > described in the CPU nodes...
> > 
> 
> I was wondering the same. I had nightmares about this one.
> If there's any better way... please, let me know!
> 

Let's see if Viresh has any good suggestions, otherwise let's stick with
this for now.

> 
> P.S.: There is a v5 of this (and CPR3) set(s) that I had sent immediately
> after this v4, back in January, addressing the big abuse of the OPP API that
> is present in the v4 (this) version of the driver.
> 

May I ask for you to incorporate the changes I pointed out here and post
a v6 instead of me re-reviewing v5? I'll make sure to prioritize the
next round.

Thanks,
Bjorn


Re: [PATCH v4 6/7] cpufreq: qcom-hw: Allow getting the maximum transition latency for OPPs

2021-04-19 Thread Bjorn Andersson
On Tue 19 Jan 11:45 CST 2021, AngeloGioacchino Del Regno wrote:

> In order to fine-tune the frequency scaling from various governors,
> allow to set a maximum transition latency from OPPs, which may be
> different depending on the SoC.
> 

Acked-by: Bjorn Andersson 

Regards,
Bjorn

> Signed-off-by: AngeloGioacchino Del Regno 
> 
> ---
>  drivers/cpufreq/qcom-cpufreq-hw.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c 
> b/drivers/cpufreq/qcom-cpufreq-hw.c
> index a92959bb7b50..5f67da796f6c 100644
> --- a/drivers/cpufreq/qcom-cpufreq-hw.c
> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
> @@ -1401,6 +1401,7 @@ static int qcom_cpufreq_hw_cpu_init(struct 
> cpufreq_policy *policy)
>   void __iomem *base;
>   struct qcom_cpufreq_data *data;
>   const char *fdom_resname;
> + unsigned int transition_latency;
>   int cpu_count, index, ret;
>  
>   cpu_dev = get_cpu_device(policy->cpu);
> @@ -1478,6 +1479,12 @@ static int qcom_cpufreq_hw_cpu_init(struct 
> cpufreq_policy *policy)
>   goto error;
>   }
>  
> + transition_latency = dev_pm_opp_get_max_transition_latency(cpu_dev);
> + if (!transition_latency)
> + transition_latency = CPUFREQ_ETERNAL;
> +
> + policy->cpuinfo.transition_latency = transition_latency;
> +
>   dev_pm_opp_of_register_em(cpu_dev, policy->cpus);
>  
>   if (policy_has_boost_freq(policy)) {
> -- 
> 2.30.0
> 


Re: [PATCH v4 2/7] cpufreq: blacklist MSM8998 in cpufreq-dt-platdev

2021-04-19 Thread Bjorn Andersson
On Tue 19 Jan 11:45 CST 2021, AngeloGioacchino Del Regno wrote:

> Add the MSM8998 to the blacklist since the CPU scaling is handled
> out of this.
> 

Reviewed-by: Bjorn Andersson 

Although I presume this could be squashed with the previous patch...

Regards,
Bjorn

> Signed-off-by: AngeloGioacchino Del Regno 
> 
> ---
>  drivers/cpufreq/cpufreq-dt-platdev.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c 
> b/drivers/cpufreq/cpufreq-dt-platdev.c
> index f82f4ec17ff2..061de3031787 100644
> --- a/drivers/cpufreq/cpufreq-dt-platdev.c
> +++ b/drivers/cpufreq/cpufreq-dt-platdev.c
> @@ -133,6 +133,7 @@ static const struct of_device_id blacklist[] __initconst 
> = {
>  
>   { .compatible = "qcom,apq8096", },
>   { .compatible = "qcom,msm8996", },
> + { .compatible = "qcom,msm8998", },
>   { .compatible = "qcom,qcs404", },
>   { .compatible = "qcom,sc7180", },
>   { .compatible = "qcom,sdm630", },
> -- 
> 2.30.0
> 


Re: [PATCH v4 1/7] cpufreq: blacklist SDM630/636/660 in cpufreq-dt-platdev

2021-04-19 Thread Bjorn Andersson
On Tue 19 Jan 11:45 CST 2021, AngeloGioacchino Del Regno wrote:

> Add the SDM630, SDM636 and SDM660 to the blacklist since the CPU
> scaling is handled out of this.
> 

Reviewed-by: Bjorn Andersson 

> Signed-off-by: AngeloGioacchino Del Regno 
> 
> ---
>  drivers/cpufreq/cpufreq-dt-platdev.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c 
> b/drivers/cpufreq/cpufreq-dt-platdev.c
> index bd2db0188cbb..f82f4ec17ff2 100644
> --- a/drivers/cpufreq/cpufreq-dt-platdev.c
> +++ b/drivers/cpufreq/cpufreq-dt-platdev.c
> @@ -135,6 +135,9 @@ static const struct of_device_id blacklist[] __initconst 
> = {
>   { .compatible = "qcom,msm8996", },
>   { .compatible = "qcom,qcs404", },
>   { .compatible = "qcom,sc7180", },
> + { .compatible = "qcom,sdm630", },
> + { .compatible = "qcom,sdm636", },
> + { .compatible = "qcom,sdm660", },
>   { .compatible = "qcom,sdm845", },
>  
>   { .compatible = "st,stih407", },
> -- 
> 2.30.0
> 


Re: [PATCH v4 4/7] dt-bindings: cpufreq: cpufreq-qcom-hw: Convert to YAML bindings

2021-04-19 Thread Bjorn Andersson
On Tue 19 Jan 11:45 CST 2021, AngeloGioacchino Del Regno wrote:
[..]
> diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml 
> b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml
> new file mode 100644
> index ..bc81b6203e27
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml
> @@ -0,0 +1,204 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/cpufreq/cpufreq-qcom-hw.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Technologies, Inc. CPUFREQ
> +
> +maintainers:
> +  - Manivannan Sadhasivam 
> +

Mani, I presume you're still up for the maintainership of this binding?

> +description: |
> +
> +  CPUFREQ HW is a hardware engine used by some Qualcomm Technologies, Inc. 
> (QTI)
> +  SoCs to manage frequency in hardware. It is capable of controlling 
> frequency
> +  for multiple clusters.
[..]
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names

This is optional in the .txt version and as such I would like you to
make this required in a separate patch following the yaml conversion.

Apart from that I think this looks good.

> +  - clocks
> +  - clock-names
> +  - '#freq-domain-cells'
> +

Regards,
Bjorn


Re: [PATCH v4 5/7] cpufreq: qcom-hw: Implement CPRh aware OSM programming

2021-04-19 Thread Bjorn Andersson
On Wed 14 Apr 20:31 CDT 2021, Taniya Das wrote:

> 
> On 4/13/2021 9:19 AM, Viresh Kumar wrote:
> > On 12-04-21, 15:01, Taniya Das wrote:
> > > Technically the HW we are trying to program here differs in terms of
> > > clocking, the LUT definitions and many more. It will definitely make
> > > debugging much more troublesome if we try to accommodate multiple 
> > > versions of
> > > CPUFREQ-HW in the same code.
> > > 
> > > Thus to keep it simple, easy to read, debug, the suggestion is to keep it
> > > with "v1" tag as the OSM version we are trying to put here is from OSM1.0.
> > 
> > That is a valid point and is always a case with so many drivers. What
> > I am concerned about is how much code is common across versions, if it
> > is 5-70%, or more, then we should definitely share, arrange to have
> > callbacks or ops per version and call them in a generic fashion instead
> > of writing a new driver. This is what's done across
> > drivers/frameworks, etc.
> > 
> 
> The code sharing here between versions should be very minimal as most
> portion of the code here in V1 would focus on programming to prepare the LUT
> to be further read by the driver, the programming in itself is huge for v1.
> I am okay if you move the v1 in a different file and invoke based on
> version.
> 

The initialization of the hardware certainly is a large chunk of code,
but once initialized it's the same driver. Your argument that this new
code makes it harder to debug things doesn't seem relevant, as the old
support doesn't seem to affect the code paths used on the modern
version.

Creating a new driver and picking it by compatible is certainly possible
to do, but looking at Angelo's patch it seems like it would contain 100%
of what's in qcom-cpufreq-hw today.

As such, I like the approach suggested in this series.

Regards,
Bjorn


Re: [PATCH v4 5/7] cpufreq: qcom-hw: Implement CPRh aware OSM programming

2021-04-19 Thread Bjorn Andersson
On Tue 19 Jan 11:45 CST 2021, AngeloGioacchino Del Regno wrote:

> On new SoCs (SDM845 onwards) the Operating State Manager (OSM) is
> being programmed in the bootloader and write-protected by the
> hypervisor, leaving to the OS read-only access to some of its
> registers (in order to read the Lookup Tables and also some
> status registers) and write access to the p-state register, for
> for the OS to request a specific performance state to trigger a
> DVFS switch on the CPU through the OSM hardware.
> 
> On old SoCs though (MSM8998, SDM630/660 and variants), the
> bootloader will *not* initialize the OSM (and the CPRh, as it
> is a requirement for it) before booting the OS, making any
> request to trigger a performance state change ineffective, as
> the hardware doesn't have any Lookup Table, nor is storing any
> parameter to trigger a DVFS switch. In this case, basically all
> of the OSM registers are *not* write protected for the OS, even
> though some are - but write access is granted through SCM calls.
> 
> This commit introduces support for OSM programming, which has to
> be done on these old SoCs that were distributed (almost?) always
> with a bootloader that does not do any CPRh nor OSM init before
> booting the kernel.
> In order to program the OSM on these SoCs, it is necessary to
> fullfill a "special" requirement: the Core Power Reduction
> Hardened (CPRh) hardware block must be initialized, as the OSM
> is "talking" to it in order to perform the Voltage part of DVFS;
> here, we are calling initialization of this through Linux generic
> power domains, specifically by requesting a genpd attach from the
> qcom-cpufreq-hw driver, which will give back voltages associated
> to each CPU frequency that has been declared in the OPPs, scaled
> and interpolated with the previous one, and will also give us
> parameters for the Array Power Mux (APM) and mem-acc, in order
> for this driver to be then able to generate the Lookup Tables
> that will be finally programmed to the OSM hardware.
> 
> After writing the parameters to the OSM and enabling it, all the
> programming work will never happen anymore until a OS reboot, so
> all of the allocations and "the rest" will be disposed-of: this
> is done mainly to leave the code that was referred only to the
> new SoCs intact, as to also emphasize on the fact that the OSM
> HW is, in the end, the exact same; apart some register offsets
> that are slightly different, the entire logic is the same.
> 
> This also adds the parameters to support CPU scaling on SDM630
> and MSM8998.
> 

Thank you for the patch Angelo and sorry for postponing the review for
so long. I'm now convinced that your approach of extending this driver
is the right one.

Some comments on the implementation though.

@Viresh, do you have any suggestion regarding my last comment?

> Signed-off-by: AngeloGioacchino Del Regno 
> 
> ---
>  drivers/cpufreq/qcom-cpufreq-hw.c | 1240 -
>  1 file changed, 1208 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c 
> b/drivers/cpufreq/qcom-cpufreq-hw.c
> index acc645b85e79..a92959bb7b50 100644
> --- a/drivers/cpufreq/qcom-cpufreq-hw.c
> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
> @@ -1,33 +1,256 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /*
>   * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + *
> + * OSM hardware initial programming
> + * Copyright (C) 2020, AngeloGioacchino Del Regno
> + * 
>   */
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
> +#include 
>  
>  #define LUT_MAX_ENTRIES  40U
> -#define LUT_SRC  GENMASK(31, 30)
> +#define LUT_SRC_845  GENMASK(31, 30)
> +#define LUT_SRC_8998 GENMASK(27, 26)
> +#define LUT_PLL_DIV  GENMASK(25, 24)
>  #define LUT_L_VALGENMASK(7, 0)
>  #define LUT_CORE_COUNT   GENMASK(18, 16)
> +#define LUT_VOLT_VC  GENMASK(21, 16)
>  #define LUT_VOLT GENMASK(11, 0)
> -#define CLK_HW_DIV   2
>  #define LUT_TURBO_IND1
> +#define OSM_BOOT_TIME_US 5
> +
> +#define CYCLE_COUNTER_CLK_RATIO  GENMASK(5, 1)
> +#define OSM_XO_RATIO_VAL (10 - 1)
> +#define CYCLE_COUNTER_USE_XO_EDGEBIT(8)
> +
> +/* FSM Boost Control */
> +#define CC_BOOST_EN  BIT(0)
> +#define PS_BOOST_EN  BIT(1)
> +#define DCVS_BOOST_ENBIT(2)
> +#define BOOST_TIMER_REG_HI   GENMASK(31, 16)
> +#define BOOST_TIMER_REG_LO   GENMASK(15, 0)
> +
> +#define PLL_WAIT_LOCK_TIME_NS2000
> +#define SAFE_FREQ_WAIT_NS1000
> +#define DEXT_DECREMENT_WAIT_NS   200
> +
> +#define BOOST_SYNC_DELAY 5
> +
> +#define 

Re: [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier

2021-04-19 Thread Bjorn Andersson
On Fri 26 Feb 03:55 CST 2021, Sai Prakash Ranjan wrote:

> Adreno(GPU) SMMU and APSS(Application Processor SubSystem) SMMU
> both implement "arm,mmu-500" in some QTI SoCs and to run through
> adreno smmu specific implementation such as enabling split pagetables
> support, we need to match the "qcom,adreno-smmu" compatible first
> before apss smmu or else we will be running apps smmu implementation
> for adreno smmu and the additional features for adreno smmu is never
> set. For ex: we have "qcom,sc7280-smmu-500" compatible for both apps
> and adreno smmu implementing "arm,mmu-500", so the adreno smmu
> implementation is never reached because the current sequence checks
> for apps smmu compatible(qcom,sc7280-smmu-500) first and runs that
> specific impl and we never reach adreno smmu specific implementation.
> 
> Suggested-by: Akhil P Oommen 
> Signed-off-by: Sai Prakash Ranjan 

Sorry for taking my time thinking about this.

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> ---
>  drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 12 +---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c 
> b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index bea3ee0dabc2..03f048aebb80 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -345,11 +345,17 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct 
> arm_smmu_device *smmu)
>  {
>   const struct device_node *np = smmu->dev->of_node;
>  
> - if (of_match_node(qcom_smmu_impl_of_match, np))
> - return qcom_smmu_create(smmu, _smmu_impl);
> -
> + /*
> +  * Do not change this order of implementation, i.e., first adreno
> +  * smmu impl and then apss smmu since we can have both implementing
> +  * arm,mmu-500 in which case we will miss setting adreno smmu specific
> +  * features if the order is changed.
> +  */
>   if (of_device_is_compatible(np, "qcom,adreno-smmu"))
>   return qcom_smmu_create(smmu, _adreno_smmu_impl);
>  
> + if (of_match_node(qcom_smmu_impl_of_match, np))
> + return qcom_smmu_create(smmu, _smmu_impl);
> +
>   return smmu;
>  }
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
> 


Re: [PATCH V3 2/4] soc: qcom: dcc:Add driver support for Data Capture and Compare unit(DCC)

2021-04-19 Thread Bjorn Andersson
On Mon 19 Apr 05:32 CDT 2021, schow...@codeaurora.org wrote:

> On 2021-04-15 12:01, Felipe Balbi wrote:
> > Hi,
> > 
> > Souradeep Chowdhury  writes:
> > > diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
> > > index ad675a6..e7f0ccb 100644
> > > --- a/drivers/soc/qcom/Makefile
> > > +++ b/drivers/soc/qcom/Makefile
> > > @@ -1,19 +1,22 @@
> > >  # SPDX-License-Identifier: GPL-2.0
> > >  CFLAGS_rpmh-rsc.o := -I$(src)
> > >  obj-$(CONFIG_QCOM_AOSS_QMP) +=   qcom_aoss.o
> > > -obj-$(CONFIG_QCOM_GENI_SE) +=qcom-geni-se.o
> > > +obj-$(CONFIG_QCOM_APR) += apr.o
> > >  obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o
> > >  obj-$(CONFIG_QCOM_CPR)   += cpr.o
> > > +obj-$(CONFIG_QCOM_DCC) += dcc.o
> > > +obj-$(CONFIG_QCOM_GENI_SE) +=   qcom-geni-se.o
> > >  obj-$(CONFIG_QCOM_GSBI)  +=  qcom_gsbi.o
> > > +obj-$(CONFIG_QCOM_KRYO_L2_ACCESSORS) += kryo-l2-accessors.o
> > > +obj-$(CONFIG_QCOM_LLCC) += llcc-qcom.o
> > >  obj-$(CONFIG_QCOM_MDT_LOADER)+= mdt_loader.o
> > >  obj-$(CONFIG_QCOM_OCMEM) += ocmem.o
> > >  obj-$(CONFIG_QCOM_PDR_HELPERS)   += pdr_interface.o
> > >  obj-$(CONFIG_QCOM_QMI_HELPERS)   += qmi_helpers.o
> > > -qmi_helpers-y+= qmi_encdec.o qmi_interface.o
> > >  obj-$(CONFIG_QCOM_RMTFS_MEM) += rmtfs_mem.o
> > >  obj-$(CONFIG_QCOM_RPMH)  += qcom_rpmh.o
> > > -qcom_rpmh-y  += rpmh-rsc.o
> > > -qcom_rpmh-y  += rpmh.o
> > > +obj-$(CONFIG_QCOM_RPMHPD) += rpmhpd.o
> > > +obj-$(CONFIG_QCOM_RPMPD) += rpmpd.o
> > >  obj-$(CONFIG_QCOM_SMD_RPM)   += smd-rpm.o
> > >  obj-$(CONFIG_QCOM_SMEM) +=   smem.o
> > >  obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
> > > @@ -21,8 +24,6 @@ obj-$(CONFIG_QCOM_SMP2P)+= smp2p.o
> > >  obj-$(CONFIG_QCOM_SMSM)  += smsm.o
> > >  obj-$(CONFIG_QCOM_SOCINFO)   += socinfo.o
> > >  obj-$(CONFIG_QCOM_WCNSS_CTRL) += wcnss_ctrl.o
> > > -obj-$(CONFIG_QCOM_APR) += apr.o
> > > -obj-$(CONFIG_QCOM_LLCC) += llcc-qcom.o
> > > -obj-$(CONFIG_QCOM_RPMHPD) += rpmhpd.o
> > > -obj-$(CONFIG_QCOM_RPMPD) += rpmpd.o
> > > -obj-$(CONFIG_QCOM_KRYO_L2_ACCESSORS) +=  kryo-l2-accessors.o
> > > +qmi_helpers-y   += qmi_encdec.o qmi_interface.o
> > > +qcom_rpmh-y += rpmh-rsc.o
> > > +qcom_rpmh-y += rpmh.o
> > 
> > why so many changes?
> 
> This has been accidentally sorted based on the config names. Will be fixing
> this in next version of the patch.
> 
> > 
> > > diff --git a/drivers/soc/qcom/dcc.c b/drivers/soc/qcom/dcc.c
> > > new file mode 100644
> > > index 000..fcd5580
> > > --- /dev/null
> > > +++ b/drivers/soc/qcom/dcc.c
> > > @@ -0,0 +1,1539 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * Copyright (c) 2015-2021, The Linux Foundation. All rights
> > > reserved.
> > > + */
> > > +
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +
> > > +
> > 
> > one blank line is enough
> 
> Ack
> 
> > 
> > > +#define TIMEOUT_US   100
> > > +
> > > +#define dcc_writel(drvdata, val, off)
> > > \
> > > + writel((val), drvdata->base + dcc_offset_conv(drvdata, off))
> > > +#define dcc_readl(drvdata, off)  
> > > \
> > > + readl(drvdata->base + dcc_offset_conv(drvdata, off))
> > > +
> > > +#define dcc_sram_readl(drvdata, off) 
> > > \
> > > + readl(drvdata->ram_base + off)
> > 
> > this would be probably be better as static inlines.
> 
> These are simple read and write operations used in the driver
> which just calls the generic writel and readl function.
> That's why macros have been used here to lesson the overhead
> of an extra function call.

The compiler will realize that your static dcc_sram_readl() is cheaper
to inline than call and do so for you. So you can expect that there's no
difference in the output from the compiler, and if there is then the
compiler knows something that you're overlooking.

Regards,
Bjorn


Re: [Patch v2 6/7] crypto: qce: common: Add support for AEAD algorithms

2021-04-18 Thread Bjorn Andersson
On Sat 17 Apr 08:25 CDT 2021, Thara Gopinath wrote:

> Add register programming sequence for enabling AEAD
> algorithms on the Qualcomm crypto engine.
> 
> Signed-off-by: Thara Gopinath 
> ---
> 
> v1->v2:
>   - Minor fixes like removing not needed initializing of variables
> and using bool values in lieu of 0 and 1 as pointed out by Bjorn.
>   - Introduced qce_be32_to_cpu_array which converts the u8 string in big
> endian order to array of u32 and returns back total number of words,
> as per Bjorn's review comments. Presently this function is used only 
> by
> qce_setup_regs_aead to format keys, iv and nonce. cipher and hash 
> algorithms can be made to use this function as a separate clean up 
> patch.

Thanks for reworking the patch Thara, I think it looks much more
reasonable now, just a few small questions/nits below.

> 
>  drivers/crypto/qce/common.c | 164 +++-
>  1 file changed, 162 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/crypto/qce/common.c b/drivers/crypto/qce/common.c
> index 7b3d6caec1b2..ffbf866842a3 100644
> --- a/drivers/crypto/qce/common.c
> +++ b/drivers/crypto/qce/common.c
> @@ -15,6 +15,16 @@
>  #include "core.h"
>  #include "regs-v5.h"
>  #include "sha.h"
> +#include "aead.h"
> +
> +static const u32 std_iv_sha1[SHA256_DIGEST_SIZE / sizeof(u32)] = {
> + SHA1_H0, SHA1_H1, SHA1_H2, SHA1_H3, SHA1_H4, 0, 0, 0
> +};
> +
> +static const u32 std_iv_sha256[SHA256_DIGEST_SIZE / sizeof(u32)] = {
> + SHA256_H0, SHA256_H1, SHA256_H2, SHA256_H3,
> + SHA256_H4, SHA256_H5, SHA256_H6, SHA256_H7
> +};
>  
>  static inline u32 qce_read(struct qce_device *qce, u32 offset)
>  {
> @@ -76,6 +86,21 @@ void qce_cpu_to_be32p_array(__be32 *dst, const u8 *src, 
> unsigned int len)
>   }
>  }
>  
> +static unsigned int qce_be32_to_cpu_array(u32 *dst, const u8 *src, unsigned 
> int len)
> +{
> + __be32 *d = (__be32 *)dst;
> + const u8 *s = src;
> + unsigned int n;
> +
> + n = len / sizeof(u32);
> + for (; n > 0; n--) {
> + *d = cpu_to_be32p((const __u32 *)s);

The output is CPU endian, so this should be be32_to_cpup()

That also means that 'd' is u32 and you don't have to play tricks and
cast dst to a __be32*.

> + s += sizeof(u32);
> + d++;
> + }
> + return DIV_ROUND_UP(len, sizeof(u32));
> +}
> +
>  static void qce_setup_config(struct qce_device *qce)
>  {
>   u32 config;
> @@ -96,7 +121,7 @@ static inline void qce_crypto_go(struct qce_device *qce, 
> bool result_dump)
>   qce_write(qce, REG_GOPROC, BIT(GO_SHIFT));
>  }
>  
> -#ifdef CONFIG_CRYPTO_DEV_QCE_SHA
> +#if defined(CONFIG_CRYPTO_DEV_QCE_SHA) || defined(CONFIG_CRYPTO_DEV_QCE_AEAD)
>  static u32 qce_auth_cfg(unsigned long flags, u32 key_size, u32 auth_size)
>  {
>   u32 cfg = 0;
> @@ -139,7 +164,9 @@ static u32 qce_auth_cfg(unsigned long flags, u32 
> key_size, u32 auth_size)
>  
>   return cfg;
>  }
> +#endif
>  
> +#ifdef CONFIG_CRYPTO_DEV_QCE_SHA
>  static int qce_setup_regs_ahash(struct crypto_async_request *async_req)
>  {
>   struct ahash_request *req = ahash_request_cast(async_req);
> @@ -225,7 +252,7 @@ static int qce_setup_regs_ahash(struct 
> crypto_async_request *async_req)
>  }
>  #endif
>  
> -#ifdef CONFIG_CRYPTO_DEV_QCE_SKCIPHER
> +#if defined(CONFIG_CRYPTO_DEV_QCE_SKCIPHER) || 
> defined(CONFIG_CRYPTO_DEV_QCE_AEAD)
>  static u32 qce_encr_cfg(unsigned long flags, u32 aes_key_size)
>  {
>   u32 cfg = 0;
> @@ -271,7 +298,9 @@ static u32 qce_encr_cfg(unsigned long flags, u32 
> aes_key_size)
>  
>   return cfg;
>  }
> +#endif
>  
> +#ifdef CONFIG_CRYPTO_DEV_QCE_SKCIPHER
>  static void qce_xts_swapiv(__be32 *dst, const u8 *src, unsigned int ivsize)
>  {
>   u8 swap[QCE_AES_IV_LENGTH];
> @@ -386,6 +415,133 @@ static int qce_setup_regs_skcipher(struct 
> crypto_async_request *async_req)
>  }
>  #endif
>  
> +#ifdef CONFIG_CRYPTO_DEV_QCE_AEAD
> +static int qce_setup_regs_aead(struct crypto_async_request *async_req)
> +{
> + struct aead_request *req = aead_request_cast(async_req);
> + struct qce_aead_reqctx *rctx = aead_request_ctx(req);
> + struct qce_aead_ctx *ctx = crypto_tfm_ctx(async_req->tfm);
> + struct qce_alg_template *tmpl = to_aead_tmpl(crypto_aead_reqtfm(req));
> + struct qce_device *qce = tmpl->qce;
> + u32 enckey[QCE_MAX_CIPHER_KEY_SIZE / sizeof(u32)] = {0};
> + u32 enciv[QCE_MAX_IV_SIZE / sizeof(u32)] = {0};
> + u32 authkey[QCE_SHA_HMAC_KEY_SIZE / sizeof(u32)] = {0};
> + u32 authiv[SHA256_DIGEST_SIZE / sizeof(u32)] = {0};
> + u32 authnonce[QCE_MAX_NONCE / sizeof(u32)] = {0};
> + unsigned int enc_keylen = ctx->enc_keylen;
> + unsigned int auth_keylen = ctx->auth_keylen;
> + unsigned int enc_ivsize = rctx->ivsize;
> + unsigned int auth_ivsize;
> + unsigned int enckey_words, enciv_words;
> + unsigned int authkey_words, authiv_words, authnonce_words;
> + 

Re: [Patch v2 3/7] crypto: qce: Add mode for rfc4309

2021-04-18 Thread Bjorn Andersson
On Sat 17 Apr 08:24 CDT 2021, Thara Gopinath wrote:

> rf4309 is the specification that uses aes ccm algorithms with IPsec
> security packets. Add a submode to identify rfc4309 ccm(aes) algorithm
> in the crypto driver.
> 

Reviewed-by: Bjorn Andersson 

> Signed-off-by: Thara Gopinath 
> ---
> 
> v1->v2:
>   - Moved up the QCE_ENCRYPT AND QCE_DECRYPT bit positions so that
> addition of other algorithms in future will not affect these
> macros.
> 
>  drivers/crypto/qce/common.h | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/crypto/qce/common.h b/drivers/crypto/qce/common.h
> index 3bc244bcca2d..b135440bf72b 100644
> --- a/drivers/crypto/qce/common.h
> +++ b/drivers/crypto/qce/common.h
> @@ -51,9 +51,11 @@
>  #define QCE_MODE_CCM BIT(12)
>  #define QCE_MODE_MASKGENMASK(12, 8)
>  
> +#define QCE_MODE_CCM_RFC4309 BIT(13)
> +
>  /* cipher encryption/decryption operations */
> -#define QCE_ENCRYPT  BIT(13)
> -#define QCE_DECRYPT  BIT(14)
> +#define QCE_ENCRYPT  BIT(30)
> +#define QCE_DECRYPT  BIT(31)
>  
>  #define IS_DES(flags)(flags & QCE_ALG_DES)
>  #define IS_3DES(flags)   (flags & QCE_ALG_3DES)
> @@ -73,6 +75,7 @@
>  #define IS_CTR(mode) (mode & QCE_MODE_CTR)
>  #define IS_XTS(mode) (mode & QCE_MODE_XTS)
>  #define IS_CCM(mode) (mode & QCE_MODE_CCM)
> +#define IS_CCM_RFC4309(mode) ((mode) & QCE_MODE_CCM_RFC4309)
>  
>  #define IS_ENCRYPT(dir)  (dir & QCE_ENCRYPT)
>  #define IS_DECRYPT(dir)  (dir & QCE_DECRYPT)
> -- 
> 2.25.1
> 


Re: [Patch v2 1/7] crypto: qce: common: Add MAC failed error checking

2021-04-18 Thread Bjorn Andersson
On Sat 17 Apr 08:24 CDT 2021, Thara Gopinath wrote:

> MAC_FAILED gets set in the status register if authenthication fails
> for ccm algorithms(during decryption). Add support to catch and flag
> this error.
> 

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> Signed-off-by: Thara Gopinath 
> ---
> 
> v1->v2:
>   - Split the error checking for -ENXIO and -EBADMSG into if-else clause
> so that the code is more readable as per Bjorn's review comment.
> 
>  drivers/crypto/qce/common.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/crypto/qce/common.c b/drivers/crypto/qce/common.c
> index dceb9579d87a..dd76175d5c62 100644
> --- a/drivers/crypto/qce/common.c
> +++ b/drivers/crypto/qce/common.c
> @@ -419,6 +419,8 @@ int qce_check_status(struct qce_device *qce, u32 *status)
>*/
>   if (*status & STATUS_ERRORS || !(*status & BIT(OPERATION_DONE_SHIFT)))
>   ret = -ENXIO;
> + else if (*status & BIT(MAC_FAILED_SHIFT))
> + ret = -EBADMSG;
>  
>   return ret;
>  }
> -- 
> 2.25.1
> 


Re: [PATCH v4 2/2] remoteproc: stm32: add capability to detach

2021-04-14 Thread Bjorn Andersson
On Wed 14 Apr 02:23 CDT 2021, Arnaud POULIQUEN wrote:
> On 4/13/21 11:34 PM, Bjorn Andersson wrote:
> > On Wed 31 Mar 02:33 CDT 2021, Arnaud Pouliquen wrote:
[..]
> >> +  err = mbox_send_message(ddata->mb[idx].chan,
> >> +  _data);
> > 
> > Seems I posted my comment on v1, rather than this latest version. Please
> > let me know if we should do anything about this dummy_data.
> 
> Thanks for pointing this out, you are right, the mailbox driver is stm32_ipcc
> and it only sends a signal to the remote processor.
> 
> As message can be queued by the mailbox framework using a local variable seems
> not a good option. As this code is a copy/past of the kick and stop?
> I propose to get this one and I will send a new patch to fix the usage in the
> whole driver.
> 

That works for me, I've merged the two patches.

Thanks,
Bjorn


Re: [PATCH v2] drivers: pinctrl: qcom: fix Kconfig dependency on GPIOLIB

2021-04-13 Thread Bjorn Andersson
On Tue 13 Apr 21:51 CDT 2021, Julian Braha wrote:

> When PINCTRL_MSM is enabled, and GPIOLIB is disabled,
> Kbuild gives the following warning:
> 
> WARNING: unmet direct dependencies detected for GPIOLIB_IRQCHIP
>   Depends on [n]: GPIOLIB [=n]
>   Selected by [y]:
>   - PINCTRL_MSM [=y] && PINCTRL [=y] && (ARCH_QCOM || COMPILE_TEST [=y])
> 
> This is because PINCTRL_MSM selects GPIOLIB_IRQCHIP,
> without selecting or depending on GPIOLIB, despite
> GPIOLIB_IRQCHIP depending on GPIOLIB. Having PINCTRL_MSM
> select GPIOLIB will cause a recursive dependency error.
> 

Reviewed-by: Bjorn Andersson 

> Signed-off-by: Julian Braha 
> ---
>  drivers/pinctrl/qcom/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig
> index 6853a896c476..d42ac59875ab 100644
> --- a/drivers/pinctrl/qcom/Kconfig
> +++ b/drivers/pinctrl/qcom/Kconfig
> @@ -3,7 +3,7 @@ if (ARCH_QCOM || COMPILE_TEST)
>  
>  config PINCTRL_MSM
>   tristate "Qualcomm core pin controller driver"
> - depends on QCOM_SCM || !QCOM_SCM #if QCOM_SCM=m this can't be =y
> + depends on GPIOLIB && (QCOM_SCM || !QCOM_SCM) #if QCOM_SCM=m this can't 
> be =y
>   select PINMUX
>   select PINCONF
>   select GENERIC_PINCONF
> -- 
> 2.27.0
> 


Re: [PATCH 1/2] dt-bindings: soc: qcom: Add bindings for Qualcomm Memshare service

2021-04-13 Thread Bjorn Andersson
On Sat 10 Apr 03:05 CDT 2021, Nikita Travkin wrote:

> Hi, sorry for a late reply but I couldn't answer earlier.
> 
> 30.03.2021 19:40, Rob Herring ??:
> > On Fri, Mar 19, 2021 at 10:23:20PM +0500, nikitos...@gmail.com wrote:
> >> From: Nikita Travkin 
> >>
> >> Add DT bindings for memshare: QMI service that allocates
> >> memory per remote processor request.
> >>
> >> Signed-off-by: Nikita Travkin 
> >> ---
> >>  .../bindings/soc/qcom/qcom,memshare.yaml  | 109 ++
> >>  include/dt-bindings/soc/qcom,memshare.h   |  10 ++
> >>  2 files changed, 119 insertions(+)
> >>  create mode 100644 
> >> Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml
> >>  create mode 100644 include/dt-bindings/soc/qcom,memshare.h
> >>
> >> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml 
> >> b/Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml
> >> new file mode 100644
> >> index ..ebdf128b066c
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml
> >> @@ -0,0 +1,109 @@
> >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> >> +%YAML 1.2
> >> +---
> >> +$id: "http://devicetree.org/schemas/soc/qcom/qcom,memshare.yaml#;
> >> +$schema: "http://devicetree.org/meta-schemas/core.yaml#;
> >> +
> >> +title: Qualcomm QMI Shared Memory Service
> > How many shared memory interfaces does Qcom have...
> >
> >> +
> >> +description: |
> >> +  This driver provides a QMI service that allows remote processors (like 
> >> modem)
> >> +  to request additional memory. It is used for applications like GPS in 
> >> modem.
> > If the memory region is defined in reserved-memory, how are you 
> > allocating additional memory? 
> 
> Initially remoteproc is loaded into it's own reserved-memory region
> but qcom decided that they sometimes need more memory than that.
> Memshare driver in msm8916 downstream tree seem to blindly allocate
> DMA region for every request that it gets. Additionally for those
> clients described in the DT, they do the DMA allocation on boot
> time and never free the region. They call it "guaranteed" allocation.
> 
> On msm8916 only one "guaranteed" client seem to be used so I decided
> to implement it with reserved-memory node. On newer platforms they
> seem to have more clients but I think that the driver can be easily
> extended to support dynamic allocation if someone really needs it.
> 

Is the "guaranteed" memory required to come from the reserved-memory
part of memory, or could it simply be allocated on demand as well (or
preallocated, but at a dynamic address)?

If these allocations always came from a reserved-memory region, then
adding a "qcom,memshare" compatible to the reserved-memory node itself
seems like a reasonable approach. But if dma_alloc is sufficient, and
there's cases where there's no "guaranteed" region, perhaps we should
just describe this as part of the remoteproc node (i.e. essentially
flipping the node/subnode in your current binding).


E.g. can we get away with simply adding an optional qcom,memshare-node
to the remoteproc binding and when that's present we make the Qualcomm
remoteproc drivers spawn the memshare handler and listen for requests
from that node?

> I tried to explain that in the cover letter but I think I made some
> mistake as I don't see it in the Patchwork.
> 
> >> +
> >> +maintainers:
> >> +  - Nikita Travkin 
> >> +
> >> +properties:
> >> +  compatible:
> >> +const: qcom,memshare
> >> +
> >> +  qcom,legacy-client:
> >> +$ref: /schemas/types.yaml#/definitions/phandle
> >> +description: Phandle to a memshare client node used for legacy 
> >> requests.
> >> +
> >> +  "#address-cells":
> >> +const: 1
> >> +
> >> +  "#size-cells":
> >> +const: 0
> >> +
> >> +patternProperties:
> >> +  "^.*@[0-9]+$":
> >> +type: object
> >> +
> >> +properties:
> >> +  reg:
> >> +description: Proc-ID for clients in this node.
> > What's Proc-ID?
> 
> The requests from the remote nodes contain client-id and proc-id
> that are supposed to differentiate the clients. It's possible to
> find the values in downstream DT or by observing what messages
> are received by the memshare service (I left dev_dbg logging in
> the driver for that reason)
> 
> I think I should reword it to make this more apparent, maybe
> "Proc-ID that clients in this node send."?
> 

If this is a constant for each remote and we make this a child thing of
remoteproc perhaps encode the number in the remoteproc nodes?

(We still need something in DT to state that we want a memshare for
a given platform/remoteproc)

> >
> >> +
> >> +  qcom,qrtr-node:
> >> +$ref: /schemas/types.yaml#/definitions/uint32
> >> +description: Node from which the requests are expected.
> >> +
> >> +  "#address-cells":
> >> +const: 1
> >> +
> >> +  "#size-cells":
> >> +const: 0
> >> +
> >> +patternProperties:
> >> +  "^.*@[0-9]+$":
> >> +

Re: [PATCH v3] ARM: kernel: Fix interrupted SMC calls

2021-04-13 Thread Bjorn Andersson
On Fri 26 Mar 13:22 CDT 2021, Manivannan Sadhasivam wrote:

> On Qualcomm ARM32 platforms, the SMC call can return before it has
> completed. If this occurs, the call can be restarted, but it requires
> using the returned session ID value from the interrupted SMC call.
> 
> The ARM32 SMCC code already has the provision to add platform specific
> quirks for things like this. So let's make use of it and add the
> Qualcomm specific quirk (ARM_SMCCC_QUIRK_QCOM_A6) used by the QCOM_SCM
> driver.
> 
> This change is similar to the below one added for ARM64 a while ago:
> commit 82bcd087029f ("firmware: qcom: scm: Fix interrupted SCM calls")
> 
> Without this change, the Qualcomm ARM32 platforms like SDX55 will return
> -EINVAL for SMC calls used for modem firmware loading and validation.
> 
> Signed-off-by: Manivannan Sadhasivam 

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> ---
> 
> Changes in v3:
> 
> * Rebased on top of v5.12-rc2
> * Sent to SoC list since there was no review so far apart from initial one
>   by Russel
> 
> Changes in v2:
> 
> * Preserved callee saved registers and used the registers r4, r5 which
>   are getting pushed onto the stack.
> 
>  arch/arm/kernel/asm-offsets.c |  3 +++
>  arch/arm/kernel/smccc-call.S  | 11 ++-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c
> index be8050b0c3df..70993af22d80 100644
> --- a/arch/arm/kernel/asm-offsets.c
> +++ b/arch/arm/kernel/asm-offsets.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include "signal.h"
>  
>  /*
> @@ -148,6 +149,8 @@ int main(void)
>DEFINE(SLEEP_SAVE_SP_PHYS, offsetof(struct sleep_save_sp, 
> save_ptr_stash_phys));
>DEFINE(SLEEP_SAVE_SP_VIRT, offsetof(struct sleep_save_sp, save_ptr_stash));
>  #endif
> +  DEFINE(ARM_SMCCC_QUIRK_ID_OFFS,offsetof(struct arm_smccc_quirk, id));
> +  DEFINE(ARM_SMCCC_QUIRK_STATE_OFFS, offsetof(struct arm_smccc_quirk, 
> state));
>BLANK();
>DEFINE(DMA_BIDIRECTIONAL,  DMA_BIDIRECTIONAL);
>DEFINE(DMA_TO_DEVICE,  DMA_TO_DEVICE);
> diff --git a/arch/arm/kernel/smccc-call.S b/arch/arm/kernel/smccc-call.S
> index 00664c78faca..931df62a7831 100644
> --- a/arch/arm/kernel/smccc-call.S
> +++ b/arch/arm/kernel/smccc-call.S
> @@ -3,7 +3,9 @@
>   * Copyright (c) 2015, Linaro Limited
>   */
>  #include 
> +#include 
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -27,7 +29,14 @@ UNWIND(.fnstart)
>  UNWIND(  .save   {r4-r7})
>   ldm r12, {r4-r7}
>   \instr
> - pop {r4-r7}
> + ldr r4, [sp, #36]
> + cmp r4, #0
> + beq 1f  // No quirk structure
> + ldr r5, [r4, #ARM_SMCCC_QUIRK_ID_OFFS]
> + cmp r5, #ARM_SMCCC_QUIRK_QCOM_A6
> + bne 1f  // No quirk present
> + str r6, [r4, #ARM_SMCCC_QUIRK_STATE_OFFS]
> +1:   pop {r4-r7}
>   ldr r12, [sp, #(4 * 4)]
>   stm r12, {r0-r3}
>   bx  lr
> -- 
> 2.25.1
> 


Re: [PATCH v1] drivers: pinctrl: qcom: fix Kconfig dependency on GPIOLIB

2021-04-13 Thread Bjorn Andersson
On Thu 08 Apr 03:03 CDT 2021, Linus Walleij wrote:

> On Mon, Mar 29, 2021 at 6:41 PM Julian Braha  wrote:
> >
> > On Tuesday, March 2, 2021 9:46:04 AM EDT you wrote:
> > > On Thu, Feb 25, 2021 at 9:33 AM Julian Braha  
> > > wrote:
> > >
> > > > When PINCTRL_MSM is enabled, and GPIOLIB is disabled,
> > > > Kbuild gives the following warning:
> > > >
> > > > WARNING: unmet direct dependencies detected for GPIOLIB_IRQCHIP
> > > >   Depends on [n]: GPIOLIB [=n]
> > > >   Selected by [y]:
> > > >   - PINCTRL_MSM [=y] && PINCTRL [=y] && (ARCH_QCOM || COMPILE_TEST [=y])
> > > >
> > > > This is because PINCTRL_MSM selects GPIOLIB_IRQCHIP,
> > > > without selecting or depending on GPIOLIB, despite
> > > > GPIOLIB_IRQCHIP depending on GPIOLIB. Having PINCTRL_MSM
> > > > select GPIOLIB will cause a recursive dependency error.
> > > >
> > > > Signed-off-by: Julian Braha 
> > >
> > > Does it work to just:
> > >
> > > select GPIOLIB
> > >
> > > instead?
> > >
> > > The driver needs the library so...
> > >
> > > Yours,
> > > Linus Walleij
> > >
> >
> > Hi Linus,
> >
> > Looks like I confused this patch with another one when
> > I responded last time. This config option cannot select
> > GPIOLIB, because it will cause a recursive dependency
> > error.
> >
> > Any other ideas?
> 
> No we can apply the patch as-is but let Bjorn have  a look at it first,
> I noticed he is not on the To: line of the original patch.
> 
> You may need to resend with Bjorn Andersson in the recipients.
> 

I don't see a resend of this, perhaps I'm just not good at searching
today...I dug up the patch on lore instead.

GPIOLIB is user selectable, so it makes sense to depend on it, rather
than use select. And it seems like our current defconfigs have GPIOLIB
enabled already (directly or indirectly).

So...

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn


Re: [PATCH 6/7] crypto: qce: common: Add support for AEAD algorithms

2021-04-13 Thread Bjorn Andersson
On Tue 13 Apr 17:44 CDT 2021, Thara Gopinath wrote:

> 
> 
> On 4/13/21 6:20 PM, Bjorn Andersson wrote:
> > On Tue 13 Apr 16:31 CDT 2021, Thara Gopinath wrote:
> > 
> > > 
> > > Hi Bjorn,
> > > 
> > > On 4/5/21 6:18 PM, Bjorn Andersson wrote:
> > > > On Thu 25 Feb 12:27 CST 2021, Thara Gopinath wrote:
> > > > 
> > > > > Add register programming sequence for enabling AEAD
> > > > > algorithms on the Qualcomm crypto engine.
> > > > > 
> > > > > Signed-off-by: Thara Gopinath 
> > > > > ---
> > > > >drivers/crypto/qce/common.c | 155 
> > > > > +++-
> > > > >1 file changed, 153 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/crypto/qce/common.c b/drivers/crypto/qce/common.c
> > > > > index 05a71c5ecf61..54d209cb0525 100644
> > > > > --- a/drivers/crypto/qce/common.c
> > > > > +++ b/drivers/crypto/qce/common.c
> > > > > @@ -15,6 +15,16 @@
> > > > >#include "core.h"
> > > > >#include "regs-v5.h"
> > > > >#include "sha.h"
> > > > > +#include "aead.h"
> > > > > +
> > > > > +static const u32 std_iv_sha1[SHA256_DIGEST_SIZE / sizeof(u32)] = {
> > > > > + SHA1_H0, SHA1_H1, SHA1_H2, SHA1_H3, SHA1_H4, 0, 0, 0
> > > > > +};
> > > > > +
> > > > > +static const u32 std_iv_sha256[SHA256_DIGEST_SIZE / sizeof(u32)] = {
> > > > > + SHA256_H0, SHA256_H1, SHA256_H2, SHA256_H3,
> > > > > + SHA256_H4, SHA256_H5, SHA256_H6, SHA256_H7
> > > > > +};
> > > > >static inline u32 qce_read(struct qce_device *qce, u32 offset)
> > > > >{
> > > > > @@ -96,7 +106,7 @@ static inline void qce_crypto_go(struct qce_device 
> > > > > *qce, bool result_dump)
> > > > >   qce_write(qce, REG_GOPROC, BIT(GO_SHIFT));
> > > > >}
> > > > > -#ifdef CONFIG_CRYPTO_DEV_QCE_SHA
> > > > > +#if defined(CONFIG_CRYPTO_DEV_QCE_SHA) || 
> > > > > defined(CONFIG_CRYPTO_DEV_QCE_AEAD)
> > > > >static u32 qce_auth_cfg(unsigned long flags, u32 key_size, u32 
> > > > > auth_size)
> > > > >{
> > > > >   u32 cfg = 0;
> > > > > @@ -139,7 +149,9 @@ static u32 qce_auth_cfg(unsigned long flags, u32 
> > > > > key_size, u32 auth_size)
> > > > >   return cfg;
> > > > >}
> > > > > +#endif
> > > > > +#ifdef CONFIG_CRYPTO_DEV_QCE_SHA
> > > > >static int qce_setup_regs_ahash(struct crypto_async_request 
> > > > > *async_req)
> > > > >{
> > > > >   struct ahash_request *req = ahash_request_cast(async_req);
> > > > > @@ -225,7 +237,7 @@ static int qce_setup_regs_ahash(struct 
> > > > > crypto_async_request *async_req)
> > > > >}
> > > > >#endif
> > > > > -#ifdef CONFIG_CRYPTO_DEV_QCE_SKCIPHER
> > > > > +#if defined(CONFIG_CRYPTO_DEV_QCE_SKCIPHER) || 
> > > > > defined(CONFIG_CRYPTO_DEV_QCE_AEAD)
> > > > >static u32 qce_encr_cfg(unsigned long flags, u32 aes_key_size)
> > > > >{
> > > > >   u32 cfg = 0;
> > > > > @@ -271,7 +283,9 @@ static u32 qce_encr_cfg(unsigned long flags, u32 
> > > > > aes_key_size)
> > > > >   return cfg;
> > > > >}
> > > > > +#endif
> > > > > +#ifdef CONFIG_CRYPTO_DEV_QCE_SKCIPHER
> > > > >static void qce_xts_swapiv(__be32 *dst, const u8 *src, unsigned 
> > > > > int ivsize)
> > > > >{
> > > > >   u8 swap[QCE_AES_IV_LENGTH];
> > > > > @@ -386,6 +400,139 @@ static int qce_setup_regs_skcipher(struct 
> > > > > crypto_async_request *async_req)
> > > > >}
> > > > >#endif
> > > > > +#ifdef CONFIG_CRYPTO_DEV_QCE_AEAD
> > > > > +static int qce_setup_regs_aead(struct crypto_async_request 
> > > > > *async_req)
> > > > > +{
> > > > > + struct aead_request *req = aead_request_cast(async_req);
> > > > > + struct qce_aead_reqctx *rctx = aead_request_ctx(req);
> > 

Re: [PATCH v2 1/2] arm64: dts: qcom: Add "dmic_clk_en" for sc7180-trogdor-coachz

2021-04-13 Thread Bjorn Andersson
On Mon 12 Apr 18:16 CDT 2021, Doug Anderson wrote:

> Bjorn,
> 
> On Mon, Mar 15, 2021 at 1:39 PM Douglas Anderson  
> wrote:
> >
> > This was present downstream. Add upstream too. NOTE: upstream I
> > managed to get some sort of halfway state and got one pinctrl entry in
> > the coachz-r1 device tree. Remove that as part of this since it's now
> > in the dtsi.
> >
> > Cc: Srinivasa Rao Mandadapu 
> > Cc: Ajit Pandey 
> > Cc: Judy Hsiao 
> > Cc: Cheng-Yi Chiang 
> > Cc: Stephen Boyd 
> > Cc: Matthias Kaehlcke 
> > Signed-off-by: Douglas Anderson 
> > ---
> > This applies atop the patch ("arm64: dts: qcom: Add sound node for
> > sc7180-trogdor-coachz") [1].
> >
> > NOTE: downstream this property was present in each of the board
> > revisions. There's actually no longer any reason for this and I'll
> > shortly post a downstream patch to fix this.
> >
> > [1] 
> > https://lore.kernel.org/r/20210313054654.11693-3-sriva...@codeaurora.org/
> >
> > Changes in v2:
> > - Remove the pinctrl from the -r1
> >
> >  .../boot/dts/qcom/sc7180-trogdor-coachz-r1.dts   | 13 -
> >  .../boot/dts/qcom/sc7180-trogdor-coachz.dtsi | 16 
> >  2 files changed, 16 insertions(+), 13 deletions(-)
> 
> I guess this patch missed the boat for 5.13? Can it get queued up for
> 5.14 whenever that happens?
> 

Meh, I scraped the inbox and thought I got everything that was ready
picked up. I'll check with Arnd, otherwise it's staged for v5.14 now.

Regards,
Bjorn


Re: [PATCH 6/7] crypto: qce: common: Add support for AEAD algorithms

2021-04-13 Thread Bjorn Andersson
On Tue 13 Apr 17:27 CDT 2021, Thara Gopinath wrote:

> 
> 
> On 4/5/21 6:18 PM, Bjorn Andersson wrote:
> > On Thu 25 Feb 12:27 CST 2021, Thara Gopinath wrote:
> > 
> > > Add register programming sequence for enabling AEAD
> > > algorithms on the Qualcomm crypto engine.
> > > 
> > > Signed-off-by: Thara Gopinath 
> > > ---
> > >   drivers/crypto/qce/common.c | 155 +++-
> > >   1 file changed, 153 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/crypto/qce/common.c b/drivers/crypto/qce/common.c
> > > index 05a71c5ecf61..54d209cb0525 100644
> > > --- a/drivers/crypto/qce/common.c
> > > +++ b/drivers/crypto/qce/common.c
> > > @@ -15,6 +15,16 @@
> > >   #include "core.h"
> > >   #include "regs-v5.h"
> > >   #include "sha.h"
> > > +#include "aead.h"
> > > +
> > > +static const u32 std_iv_sha1[SHA256_DIGEST_SIZE / sizeof(u32)] = {
> > > + SHA1_H0, SHA1_H1, SHA1_H2, SHA1_H3, SHA1_H4, 0, 0, 0
> > > +};
> > > +
> > > +static const u32 std_iv_sha256[SHA256_DIGEST_SIZE / sizeof(u32)] = {
> > > + SHA256_H0, SHA256_H1, SHA256_H2, SHA256_H3,
> > > + SHA256_H4, SHA256_H5, SHA256_H6, SHA256_H7
> > > +};
> > >   static inline u32 qce_read(struct qce_device *qce, u32 offset)
> > >   {
> > > @@ -96,7 +106,7 @@ static inline void qce_crypto_go(struct qce_device 
> > > *qce, bool result_dump)
> > >   qce_write(qce, REG_GOPROC, BIT(GO_SHIFT));
> > >   }
> > > -#ifdef CONFIG_CRYPTO_DEV_QCE_SHA
> > > +#if defined(CONFIG_CRYPTO_DEV_QCE_SHA) || 
> > > defined(CONFIG_CRYPTO_DEV_QCE_AEAD)
> > >   static u32 qce_auth_cfg(unsigned long flags, u32 key_size, u32 
> > > auth_size)
> > >   {
> > >   u32 cfg = 0;
> > > @@ -139,7 +149,9 @@ static u32 qce_auth_cfg(unsigned long flags, u32 
> > > key_size, u32 auth_size)
> > >   return cfg;
> > >   }
> > > +#endif
> > > +#ifdef CONFIG_CRYPTO_DEV_QCE_SHA
> > >   static int qce_setup_regs_ahash(struct crypto_async_request *async_req)
> > >   {
> > >   struct ahash_request *req = ahash_request_cast(async_req);
> > > @@ -225,7 +237,7 @@ static int qce_setup_regs_ahash(struct 
> > > crypto_async_request *async_req)
> > >   }
> > >   #endif
> > > -#ifdef CONFIG_CRYPTO_DEV_QCE_SKCIPHER
> > > +#if defined(CONFIG_CRYPTO_DEV_QCE_SKCIPHER) || 
> > > defined(CONFIG_CRYPTO_DEV_QCE_AEAD)
> > >   static u32 qce_encr_cfg(unsigned long flags, u32 aes_key_size)
> > >   {
> > >   u32 cfg = 0;
> > > @@ -271,7 +283,9 @@ static u32 qce_encr_cfg(unsigned long flags, u32 
> > > aes_key_size)
> > >   return cfg;
> > >   }
> > > +#endif
> > > +#ifdef CONFIG_CRYPTO_DEV_QCE_SKCIPHER
> > >   static void qce_xts_swapiv(__be32 *dst, const u8 *src, unsigned int 
> > > ivsize)
> > >   {
> > >   u8 swap[QCE_AES_IV_LENGTH];
> > > @@ -386,6 +400,139 @@ static int qce_setup_regs_skcipher(struct 
> > > crypto_async_request *async_req)
> > >   }
> > >   #endif
> > > +#ifdef CONFIG_CRYPTO_DEV_QCE_AEAD
> > > +static int qce_setup_regs_aead(struct crypto_async_request *async_req)
> > > +{
> > > + struct aead_request *req = aead_request_cast(async_req);
> > > + struct qce_aead_reqctx *rctx = aead_request_ctx(req);
> > > + struct qce_aead_ctx *ctx = crypto_tfm_ctx(async_req->tfm);
> > > + struct qce_alg_template *tmpl = to_aead_tmpl(crypto_aead_reqtfm(req));
> > > + struct qce_device *qce = tmpl->qce;
> > > + __be32 enckey[QCE_MAX_CIPHER_KEY_SIZE / sizeof(__be32)] = {0};
> > > + __be32 enciv[QCE_MAX_IV_SIZE / sizeof(__be32)] = {0};
> > > + __be32 authkey[QCE_SHA_HMAC_KEY_SIZE / sizeof(__be32)] = {0};
> > > + __be32 authiv[SHA256_DIGEST_SIZE / sizeof(__be32)] = {0};
> > > + __be32 authnonce[QCE_MAX_NONCE / sizeof(__be32)] = {0};
> > > + unsigned int enc_keylen = ctx->enc_keylen;
> > > + unsigned int auth_keylen = ctx->auth_keylen;
> > > + unsigned int enc_ivsize = rctx->ivsize;
> > > + unsigned int auth_ivsize;
> > > + unsigned int enckey_words, enciv_words;
> > > + unsigned int authkey_words, authiv_words, authnonce_words;
> > > + unsigned long flags = rctx->flags;
> > > + u32 encr_cfg = 0, auth_cfg = 0, config, totallen;
>

Re: [PATCH 6/7] crypto: qce: common: Add support for AEAD algorithms

2021-04-13 Thread Bjorn Andersson
On Tue 13 Apr 16:31 CDT 2021, Thara Gopinath wrote:

> 
> Hi Bjorn,
> 
> On 4/5/21 6:18 PM, Bjorn Andersson wrote:
> > On Thu 25 Feb 12:27 CST 2021, Thara Gopinath wrote:
> > 
> > > Add register programming sequence for enabling AEAD
> > > algorithms on the Qualcomm crypto engine.
> > > 
> > > Signed-off-by: Thara Gopinath 
> > > ---
> > >   drivers/crypto/qce/common.c | 155 +++-
> > >   1 file changed, 153 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/crypto/qce/common.c b/drivers/crypto/qce/common.c
> > > index 05a71c5ecf61..54d209cb0525 100644
> > > --- a/drivers/crypto/qce/common.c
> > > +++ b/drivers/crypto/qce/common.c
> > > @@ -15,6 +15,16 @@
> > >   #include "core.h"
> > >   #include "regs-v5.h"
> > >   #include "sha.h"
> > > +#include "aead.h"
> > > +
> > > +static const u32 std_iv_sha1[SHA256_DIGEST_SIZE / sizeof(u32)] = {
> > > + SHA1_H0, SHA1_H1, SHA1_H2, SHA1_H3, SHA1_H4, 0, 0, 0
> > > +};
> > > +
> > > +static const u32 std_iv_sha256[SHA256_DIGEST_SIZE / sizeof(u32)] = {
> > > + SHA256_H0, SHA256_H1, SHA256_H2, SHA256_H3,
> > > + SHA256_H4, SHA256_H5, SHA256_H6, SHA256_H7
> > > +};
> > >   static inline u32 qce_read(struct qce_device *qce, u32 offset)
> > >   {
> > > @@ -96,7 +106,7 @@ static inline void qce_crypto_go(struct qce_device 
> > > *qce, bool result_dump)
> > >   qce_write(qce, REG_GOPROC, BIT(GO_SHIFT));
> > >   }
> > > -#ifdef CONFIG_CRYPTO_DEV_QCE_SHA
> > > +#if defined(CONFIG_CRYPTO_DEV_QCE_SHA) || 
> > > defined(CONFIG_CRYPTO_DEV_QCE_AEAD)
> > >   static u32 qce_auth_cfg(unsigned long flags, u32 key_size, u32 
> > > auth_size)
> > >   {
> > >   u32 cfg = 0;
> > > @@ -139,7 +149,9 @@ static u32 qce_auth_cfg(unsigned long flags, u32 
> > > key_size, u32 auth_size)
> > >   return cfg;
> > >   }
> > > +#endif
> > > +#ifdef CONFIG_CRYPTO_DEV_QCE_SHA
> > >   static int qce_setup_regs_ahash(struct crypto_async_request *async_req)
> > >   {
> > >   struct ahash_request *req = ahash_request_cast(async_req);
> > > @@ -225,7 +237,7 @@ static int qce_setup_regs_ahash(struct 
> > > crypto_async_request *async_req)
> > >   }
> > >   #endif
> > > -#ifdef CONFIG_CRYPTO_DEV_QCE_SKCIPHER
> > > +#if defined(CONFIG_CRYPTO_DEV_QCE_SKCIPHER) || 
> > > defined(CONFIG_CRYPTO_DEV_QCE_AEAD)
> > >   static u32 qce_encr_cfg(unsigned long flags, u32 aes_key_size)
> > >   {
> > >   u32 cfg = 0;
> > > @@ -271,7 +283,9 @@ static u32 qce_encr_cfg(unsigned long flags, u32 
> > > aes_key_size)
> > >   return cfg;
> > >   }
> > > +#endif
> > > +#ifdef CONFIG_CRYPTO_DEV_QCE_SKCIPHER
> > >   static void qce_xts_swapiv(__be32 *dst, const u8 *src, unsigned int 
> > > ivsize)
> > >   {
> > >   u8 swap[QCE_AES_IV_LENGTH];
> > > @@ -386,6 +400,139 @@ static int qce_setup_regs_skcipher(struct 
> > > crypto_async_request *async_req)
> > >   }
> > >   #endif
> > > +#ifdef CONFIG_CRYPTO_DEV_QCE_AEAD
> > > +static int qce_setup_regs_aead(struct crypto_async_request *async_req)
> > > +{
> > > + struct aead_request *req = aead_request_cast(async_req);
> > > + struct qce_aead_reqctx *rctx = aead_request_ctx(req);
> > > + struct qce_aead_ctx *ctx = crypto_tfm_ctx(async_req->tfm);
> > > + struct qce_alg_template *tmpl = to_aead_tmpl(crypto_aead_reqtfm(req));
> > > + struct qce_device *qce = tmpl->qce;
> > > + __be32 enckey[QCE_MAX_CIPHER_KEY_SIZE / sizeof(__be32)] = {0};
> > > + __be32 enciv[QCE_MAX_IV_SIZE / sizeof(__be32)] = {0};
> > > + __be32 authkey[QCE_SHA_HMAC_KEY_SIZE / sizeof(__be32)] = {0};
> > > + __be32 authiv[SHA256_DIGEST_SIZE / sizeof(__be32)] = {0};
> > > + __be32 authnonce[QCE_MAX_NONCE / sizeof(__be32)] = {0};
> > > + unsigned int enc_keylen = ctx->enc_keylen;
> > > + unsigned int auth_keylen = ctx->auth_keylen;
> > > + unsigned int enc_ivsize = rctx->ivsize;
> > > + unsigned int auth_ivsize;
> > > + unsigned int enckey_words, enciv_words;
> > > + unsigned int authkey_words, authiv_words, authnonce_words;
> > > + unsigned long flags = rctx->flags;
> > > + u32 encr_cfg = 0, auth_cfg = 0, config, to

Re: [PATCH v4 2/2] remoteproc: stm32: add capability to detach

2021-04-13 Thread Bjorn Andersson
On Wed 31 Mar 02:33 CDT 2021, Arnaud Pouliquen wrote:

> A mechanism similar to the shutdown mailbox signal is implemented to
> detach a remote processor.
> 
> Upon detachment, a signal is sent to the remote firmware, allowing it
> to perform specific actions such as stopping rpmsg communication.
> 
> The Cortex-M hold boot is also disabled to allow the remote processor
> to restart in case of crash.
> 
> Signed-off-by: Arnaud Pouliquen 
> Reviewed-by: Mathieu Poirier 
> Tested-by: Mathieu Poirier 
> ---
>  drivers/remoteproc/stm32_rproc.c | 39 ++--
>  1 file changed, 37 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/remoteproc/stm32_rproc.c 
> b/drivers/remoteproc/stm32_rproc.c
> index 3d45f51de4d0..7353f9e7e7af 100644
> --- a/drivers/remoteproc/stm32_rproc.c
> +++ b/drivers/remoteproc/stm32_rproc.c
> @@ -28,7 +28,7 @@
>  #define RELEASE_BOOT 1
>  
>  #define MBOX_NB_VQ   2
> -#define MBOX_NB_MBX  3
> +#define MBOX_NB_MBX  4
>  
>  #define STM32_SMC_RCC0x82001000
>  #define STM32_SMC_REG_WRITE  0x1
> @@ -38,6 +38,7 @@
>  #define STM32_MBX_VQ1"vq1"
>  #define STM32_MBX_VQ1_ID 1
>  #define STM32_MBX_SHUTDOWN   "shutdown"
> +#define STM32_MBX_DETACH "detach"
>  
>  #define RSC_TBL_SIZE 1024
>  
> @@ -336,6 +337,15 @@ static const struct stm32_mbox 
> stm32_rproc_mbox[MBOX_NB_MBX] = {
>   .tx_done = NULL,
>   .tx_tout = 500, /* 500 ms time out */
>   },
> + },
> + {
> + .name = STM32_MBX_DETACH,
> + .vq_id = -1,
> + .client = {
> + .tx_block = true,
> + .tx_done = NULL,
> + .tx_tout = 200, /* 200 ms time out to detach should be 
> fair enough */
> + },
>   }
>  };
>  
> @@ -461,6 +471,25 @@ static int stm32_rproc_attach(struct rproc *rproc)
>   return stm32_rproc_set_hold_boot(rproc, true);
>  }
>  
> +static int stm32_rproc_detach(struct rproc *rproc)
> +{
> + struct stm32_rproc *ddata = rproc->priv;
> + int err, dummy_data, idx;
> +
> + /* Inform the remote processor of the detach */
> + idx = stm32_rproc_mbox_idx(rproc, STM32_MBX_DETACH);
> + if (idx >= 0 && ddata->mb[idx].chan) {
> + /* A dummy data is sent to allow to block on transmit */
> + err = mbox_send_message(ddata->mb[idx].chan,
> + _data);

Seems I posted my comment on v1, rather than this latest version. Please
let me know if we should do anything about this dummy_data.

Regards,
Bjorn

> + if (err < 0)
> + dev_warn(>dev, "warning: remote FW detach 
> without ack\n");
> + }
> +
> + /* Allow remote processor to auto-reboot */
> + return stm32_rproc_set_hold_boot(rproc, false);
> +}
> +
>  static int stm32_rproc_stop(struct rproc *rproc)
>  {
>   struct stm32_rproc *ddata = rproc->priv;
> @@ -597,7 +626,12 @@ stm32_rproc_get_loaded_rsc_table(struct rproc *rproc, 
> size_t *table_sz)
>   }
>  
>  done:
> - /* Assuming the resource table fits in 1kB is fair */
> + /*
> +  * Assuming the resource table fits in 1kB is fair.
> +  * Notice for the detach, that this 1 kB memory area has to be reserved 
> in the coprocessor
> +  * firmware for the resource table. On detach, the remoteproc core 
> re-initializes this
> +  * entire area by overwriting it with the initial values stored in 
> rproc->clean_table.
> +  */
>   *table_sz = RSC_TBL_SIZE;
>   return (struct resource_table *)ddata->rsc_va;
>  }
> @@ -607,6 +641,7 @@ static const struct rproc_ops st_rproc_ops = {
>   .start  = stm32_rproc_start,
>   .stop   = stm32_rproc_stop,
>   .attach = stm32_rproc_attach,
> + .detach = stm32_rproc_detach,
>   .kick   = stm32_rproc_kick,
>   .load   = rproc_elf_load_segments,
>   .parse_fw   = stm32_rproc_parse_fw,
> -- 
> 2.17.1
> 


Re: [PATCH V2 1/2] soc: qcom: aoss: Expose send for generic usecase

2021-04-13 Thread Bjorn Andersson
On Thu 08 Apr 23:39 CDT 2021, Deepak Kumar Singh wrote:
[..]
> +/**
> + * qmp_get() - get a qmp handle from a device
> + * @dev: client device pointer
> + *
> + * Return: handle to qmp device on success, ERR_PTR() on failure
> + */
> +struct qmp *qmp_get(struct device *dev)
> +{
> + struct platform_device *pdev;
> + struct device_node *np;
> + struct qmp *qmp;
> +
> + if (!dev || !dev->of_node)
> + return ERR_PTR(-EINVAL);
> +
> + np = of_parse_phandle(dev->of_node, "qcom,qmp", 0);
> + if (!np)
> + return ERR_PTR(-ENODEV);
> +
> + pdev = of_find_device_by_node(np);
> + if (!pdev)
> + return ERR_PTR(-EINVAL);
> +
> + qmp = platform_get_drvdata(pdev);
> + return qmp ? qmp : ERR_PTR(-EPROBE_DEFER);
> +}
> +EXPORT_SYMBOL(qmp_get);
> +
> +void qmp_put(struct platform_device *pdev)
> +{
> + platform_device_put(pdev);

I was expecting that the devres allocated struct qmp would stick around
until the struct device's release callback came. As described in my
answer to Mani yesterday I was wrong.

As such you need to make sure that struct qmp stays around until
qmp_remove() and all qmp_put() calls has been made.

I presume a reasonable way to achieve this is to not use devm to
allocate our struct qmp, add a kref to the object. So this would have to
be a kref_put() on that instead of the platform_device_put() I asked you
to implement here.

Sorry about that.


The rest of the patch looks good.

Regards,
Bjorn


Re: [PATCH 3/5] dt-bindings: remoteproc: Add the documentation for Meson AO ARC rproc

2021-04-13 Thread Bjorn Andersson
On Tue 23 Mar 17:02 CDT 2021, Martin Blumenstingl wrote:

> Hi Bjorn,
> 
> On Thu, Mar 18, 2021 at 3:55 AM Bjorn Andersson
>  wrote:
> [...]
> > > +examples:
> > > +  - |
> > > +remoteproc@1c {
> > > +  compatible= "amlogic,meson8-ao-arc", "amlogic,meson-mx-ao-arc";
> > > +  reg = <0x1c 0x8>, <0x38 0x8>;
> >
> > I'm generally not in favor of mapping "individual" registers, do you
> > know what hardware block this is part of? Can you express the whole
> > block as an single entity in your DT?
> the answer is unfortunately not easy :-)
> 
> some background information:
> Amlogic SoCs have two power domains:
> - AO (Always-On)
> - EE (Everything-Else)
> 
> AO includes (at least) one ARC core for which this remoteproc dt-binding is.
> EE includes ARM Cortex-A7/15/... cores
> 
> The AO registers can be accessed from the EE power-domain and vice versa
> 
> Following is an extract (with comments added by me) for the AO
> registers (taken from the GPL vendor kernel):
> #define AO_RTI_STATUS_REG0 ((0x00 << 10) | (0x00 << 2))
> #define AO_RTI_STATUS_REG1 ((0x00 << 10) | (0x01 << 2))
> #define AO_RTI_STATUS_REG2 ((0x00 << 10) | (0x02 << 2))
> these three are used for communication with the firmware on the AO ARC core
> I am not sure into which Linux subsystem these would fit into best
> 
> #define AO_RTI_PWR_CNTL_REG1 ((0x00 << 10) | (0x03 << 2))
> #define AO_RTI_PWR_CNTL_REG0 ((0x00 << 10) | (0x04 << 2))
> this includes various power-domains for the following functionality
> (and probably more):
> - DDR PHY I/O
> - AHB SRAM
> - video encoder/decoders
> - EE domain isolation
> 
> #define AO_RTI_PIN_MUX_REG ((0x00 << 10) | (0x05 << 2))
> first part of the pin controller registers for the "AO" bank pads
> this includes various GPIOs, UART, I2C for communication with a PMIC,
> infrared remote decoder, two PWMs, etc.
> all (known) functionality can be used by Linux as well.
> especially the UART, I2C, IR decoder and GPIOs are functionality that
> we use with Linux today - without involving the AO ARC
> remote-processor.
> 
> #define AO_WD_GPIO_REG ((0x00 << 10) | (0x06 << 2))
> (I think this is related to the watchdog being able to trigger the
> SoC's reset line, but there's no documentation on this register)
> 
> #define AO_REMAP_REG0 ((0x00 << 10) | (0x07 << 2))
> #define AO_REMAP_REG1 ((0x00 << 10) | (0x08 << 2))
> remap registers for the AO ARC remote-processor as used in this binding
> 
> #define AO_GPIO_O_EN_N ((0x00 << 10) | (0x09 << 2))
> #define AO_GPIO_I ((0x00 << 10) | (0x0A << 2))
> GPIO controller registers for the "AO" bank pads
> 
> #define AO_RTI_PULL_UP_REG ((0x00 << 10) | (0x0B << 2))
> second part of the pin controller registers for the "AO" bank pads
> 
> #define AO_RTI_WD_MARK ((0x00 << 10) | (0x0D << 2))
> again, I think this is somehow related to the watchdog but there's no
> documentation on this
> 
> #define AO_CPU_CNTL ((0x00 << 10) | (0x0E << 2))
> #define AO_CPU_STAT ((0x00 << 10) | (0x0F << 2))
> used for booting the AO ARC remote-processor
> 
> #define AO_RTI_GEN_CNTL_REG0 ((0x00 << 10) | (0x10 << 2))
> seems to be a multi purpose register as it (seems to) contains some
> reset bits (for the AO UART and RTC) - not documented
> 
> (more registers are following)
> 
> to summarize this: I think there's indeed three different sets of registers
> having one big device-tree node spanning all of these registers seems
> incorrect to me as the other IPs are independent of the AO ARC
> remote-processor.
> so the way I have done it in the original patch is the best I could
> come up with.
> 
> Please let me know what you think!
> 

I see.

Describing these kinds blocks in DT is indeed tricky, I've had
both cases where a block maps to multiple "functions" or where they
contain misc registers to be used in relation to some other block.

The prior typically lends itself to be modelled as a "simple-mfd" and
the latter as a "syscon".

So perhaps you could do a simple-mfd that spans the entire block and
then describe the remoteproc, watchdog?, pinctrl pieces as children
under that?

Regards,
Bjorn


Re: [PATCH 2/2] remoteproc: stm32: add capability to detach

2021-04-13 Thread Bjorn Andersson
On Thu 18 Mar 09:59 CDT 2021, Arnaud Pouliquen wrote:

> From: Arnaud Pouliquen 
> 
> A mechanism similar to the shutdown mailbox signal is implemented to
> detach a remote processor.
> 
> Upon detachment, a signal is sent to the remote firmware, allowing it
> to perform specific actions such as stopping RPMsg communication.
> 
> The Cortex-M hold boot is also disabled to allow the remote processor
> to restart in case of crash.
> 
> Notice that for this feature to be supported, the remote firmware 
> resource table must be stored at the beginning of a 1kB section 
> (default size provided to the remoteproc core).
> 
> This restriction should be lifted in the future by using a backup register
> to store the actual size of the resource table. 
> 
> Signed-off-by: Arnaud Pouliquen 
> ---
>  drivers/remoteproc/stm32_rproc.c | 38 ++--
>  1 file changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/remoteproc/stm32_rproc.c 
> b/drivers/remoteproc/stm32_rproc.c
> index 3d45f51de4d0..298ef5b19e27 100644
> --- a/drivers/remoteproc/stm32_rproc.c
> +++ b/drivers/remoteproc/stm32_rproc.c
> @@ -28,7 +28,7 @@
>  #define RELEASE_BOOT 1
>  
>  #define MBOX_NB_VQ   2
> -#define MBOX_NB_MBX  3
> +#define MBOX_NB_MBX  4
>  
>  #define STM32_SMC_RCC0x82001000
>  #define STM32_SMC_REG_WRITE  0x1
> @@ -38,6 +38,7 @@
>  #define STM32_MBX_VQ1"vq1"
>  #define STM32_MBX_VQ1_ID 1
>  #define STM32_MBX_SHUTDOWN   "shutdown"
> +#define STM32_MBX_DETACH "detach"
>  
>  #define RSC_TBL_SIZE 1024
>  
> @@ -336,6 +337,15 @@ static const struct stm32_mbox 
> stm32_rproc_mbox[MBOX_NB_MBX] = {
>   .tx_done = NULL,
>   .tx_tout = 500, /* 500 ms time out */
>   },
> + },
> + {
> + .name = STM32_MBX_DETACH,
> + .vq_id = -1,
> + .client = {
> + .tx_block = true,
> + .tx_done = NULL,
> + .tx_tout = 200, /* 200 ms time out to detach should be 
> fair enough */
> + },
>   }
>  };
>  
> @@ -461,6 +471,25 @@ static int stm32_rproc_attach(struct rproc *rproc)
>   return stm32_rproc_set_hold_boot(rproc, true);
>  }
>  
> +static int stm32_rproc_detach(struct rproc *rproc)
> +{
> + struct stm32_rproc *ddata = rproc->priv;
> + int err, dummy_data, idx;
> +
> + /* Inform the remote processor of the detach */
> + idx = stm32_rproc_mbox_idx(rproc, STM32_MBX_DETACH);
> + if (idx >= 0 && ddata->mb[idx].chan) {
> + /* A dummy data is sent to allow to block on transmit */
> + err = mbox_send_message(ddata->mb[idx].chan,
> + _data);

Isn't it the stm32_ipcc driver on the other side of this call? In which
case I believe "data" is ignored, and you would be able to just pass
NULL here.

As long as "data" isn't dereferenced it's probably better to send some
bugus value, than an address to this variable on the stack. If on the
other hand you pair this with one of the mailbox drivers that
dereferences "data", you should initialize it...

Apart from this, I think the patch looks good!

Regards,
Bjorn

> + if (err < 0)
> + dev_warn(>dev, "warning: remote FW detach 
> without ack\n");
> + }
> +
> + /* Allow remote processor to auto-reboot */
> + return stm32_rproc_set_hold_boot(rproc, false);
> +}
> +
>  static int stm32_rproc_stop(struct rproc *rproc)
>  {
>   struct stm32_rproc *ddata = rproc->priv;
> @@ -597,7 +626,11 @@ stm32_rproc_get_loaded_rsc_table(struct rproc *rproc, 
> size_t *table_sz)
>   }
>  
>  done:
> - /* Assuming the resource table fits in 1kB is fair */
> + /*
> +  * Assuming the resource table fits in 1kB is fair.
> +  * Notice for the detach, that this 1 kB memory area has to be reserved 
> in the coprocessor
> +  * firmware for the resource table. A clean of this whole area is done 
> on detach.
> +  */
>   *table_sz = RSC_TBL_SIZE;
>   return (struct resource_table *)ddata->rsc_va;
>  }
> @@ -607,6 +640,7 @@ static const struct rproc_ops st_rproc_ops = {
>   .start  = stm32_rproc_start,
>   .stop   = stm32_rproc_stop,
>   .attach = stm32_rproc_attach,
> + .detach = stm32_rproc_detach,
>   .kick   = stm32_rproc_kick,
>   .load   = rproc_elf_load_segments,
>   .parse_fw   = stm32_rproc_parse_fw,
> -- 
> 2.17.1
> 


Re: [PATCH 3/7] crypto: qce: Add mode for rfc4309

2021-04-13 Thread Bjorn Andersson
On Tue 13 Apr 14:30 CDT 2021, Thara Gopinath wrote:

> 
> 
> On 4/5/21 6:32 PM, Bjorn Andersson wrote:
> > On Thu 25 Feb 12:27 CST 2021, Thara Gopinath wrote:
> > 
> > > rf4309 is the specification that uses aes ccm algorithms with IPsec
> > > security packets. Add a submode to identify rfc4309 ccm(aes) algorithm
> > > in the crypto driver.
> > > 
> > > Signed-off-by: Thara Gopinath 
> > > ---
> > >   drivers/crypto/qce/common.h | 7 +--
> > >   1 file changed, 5 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/crypto/qce/common.h b/drivers/crypto/qce/common.h
> > > index 3bc244bcca2d..3ffe719b79e4 100644
> > > --- a/drivers/crypto/qce/common.h
> > > +++ b/drivers/crypto/qce/common.h
> > > @@ -51,9 +51,11 @@
> > >   #define QCE_MODE_CCMBIT(12)
> > >   #define QCE_MODE_MASK   GENMASK(12, 8)
> > > +#define QCE_MODE_CCM_RFC4309 BIT(13)
> > > +
> > >   /* cipher encryption/decryption operations */
> > > -#define QCE_ENCRYPT  BIT(13)
> > > -#define QCE_DECRYPT  BIT(14)
> > > +#define QCE_ENCRYPT  BIT(14)
> > > +#define QCE_DECRYPT  BIT(15)
> > 
> > Can't we move these further up, so that next time we want to add
> > something it doesn't require that we also move the ENC/DEC bits?
> 
> Yes I will change it to BIT(30) and BIT(31)
> 
> > 
> > >   #define IS_DES(flags)   (flags & QCE_ALG_DES)
> > >   #define IS_3DES(flags)  (flags & QCE_ALG_3DES)
> > > @@ -73,6 +75,7 @@
> > >   #define IS_CTR(mode)(mode & QCE_MODE_CTR)
> > >   #define IS_XTS(mode)(mode & QCE_MODE_XTS)
> > >   #define IS_CCM(mode)(mode & QCE_MODE_CCM)
> > > +#define IS_CCM_RFC4309(mode) ((mode) & QCE_MODE_CCM_RFC4309)
> > 
> > While leaving room for the typical macro issues, none of the other
> > macros wrap the argument in parenthesis. Please follow the style of the
> > driver, and perhaps follow up with a cleanup patch that just wraps them
> > all in parenthesis?
> 
> This does throw up a checkpatch warning if I don't wrap "mode" in
> parenthesis. How about I keep this for now and I will follow up with a clean
> up for rest of the macros later ?
> 

I don't have a problem with this approach.

Regards,
Bjorn

> > 
> > Regards,
> > Bjorn
> > 
> > >   #define IS_ENCRYPT(dir) (dir & QCE_ENCRYPT)
> > >   #define IS_DECRYPT(dir) (dir & QCE_DECRYPT)
> > > -- 
> > > 2.25.1
> > > 
> 
> -- 
> Warm Regards
> Thara


Re: [PATCH V2 1/4] input: pm8941-pwrkey: add support for PMK8350 PON_HLOS PMIC peripheral

2021-04-12 Thread Bjorn Andersson
On Thu 08 Apr 06:31 CDT 2021, satya priya wrote:

> From: David Collins 
> 
> On Qualcomm Technologies, Inc. PMIC PMK8350, the PON peripheral
> is split into two peripherals: PON_HLOS and PON_PBS.  The
> application processor only has write access to PON_HLOS which
> limits it to only receiving PON interrupts.
> 
> Add support for the PMK8350 PON_HLOS peripheral so that its
> KPDPWR_N and RESIN_N interrupts can be used to detect key
> presses.
> 

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> Signed-off-by: David Collins 
> Signed-off-by: satya priya 
> ---
> Changes in V2:
>  - No change.
> 
>  drivers/input/misc/pm8941-pwrkey.c | 103 
> ++---
>  1 file changed, 72 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/input/misc/pm8941-pwrkey.c 
> b/drivers/input/misc/pm8941-pwrkey.c
> index cf81044..10e3fc0 100644
> --- a/drivers/input/misc/pm8941-pwrkey.c
> +++ b/drivers/input/misc/pm8941-pwrkey.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /*
> - * Copyright (c) 2010-2011, Code Aurora Forum. All rights reserved.
> + * Copyright (c) 2010-2011, 2020-2021, The Linux Foundation. All rights 
> reserved.
>   * Copyright (c) 2014, Sony Mobile Communications Inc.
>   */
>  
> @@ -22,6 +22,8 @@
>  #define PON_RT_STS   0x10
>  #define  PON_KPDPWR_N_SETBIT(0)
>  #define  PON_RESIN_N_SET BIT(1)
> +#define  PON_GEN3_RESIN_N_SETBIT(6)
> +#define  PON_GEN3_KPDPWR_N_SET   BIT(7)
>  
>  #define PON_PS_HOLD_RST_CTL  0x5a
>  #define PON_PS_HOLD_RST_CTL2 0x5b
> @@ -38,8 +40,12 @@
>  #define  PON_DBC_DELAY_MASK  0x7
>  
>  struct pm8941_data {
> - unsigned int pull_up_bit;
> - unsigned int status_bit;
> + unsigned intpull_up_bit;
> + unsigned intstatus_bit;
> + boolsupports_ps_hold_poff_config;
> + boolsupports_debounce_config;
> + const char  *name;
> + const char  *phys;
>  };
>  
>  struct pm8941_pwrkey {
> @@ -231,34 +237,40 @@ static int pm8941_pwrkey_probe(struct platform_device 
> *pdev)
>  
>   input_set_capability(pwrkey->input, EV_KEY, pwrkey->code);
>  
> - pwrkey->input->name = "pm8941_pwrkey";
> - pwrkey->input->phys = "pm8941_pwrkey/input0";
> -
> - req_delay = (req_delay << 6) / USEC_PER_SEC;
> - req_delay = ilog2(req_delay);
> -
> - error = regmap_update_bits(pwrkey->regmap,
> -pwrkey->baseaddr + PON_DBC_CTL,
> -PON_DBC_DELAY_MASK,
> -req_delay);
> - if (error) {
> - dev_err(>dev, "failed to set debounce: %d\n", error);
> - return error;
> + pwrkey->input->name = pwrkey->data->name;
> + pwrkey->input->phys = pwrkey->data->phys;
> +
> + if (pwrkey->data->supports_debounce_config) {
> + req_delay = (req_delay << 6) / USEC_PER_SEC;
> + req_delay = ilog2(req_delay);
> +
> + error = regmap_update_bits(pwrkey->regmap,
> +pwrkey->baseaddr + PON_DBC_CTL,
> +PON_DBC_DELAY_MASK,
> +req_delay);
> + if (error) {
> + dev_err(>dev, "failed to set debounce: %d\n",
> + error);
> + return error;
> + }
>   }
>  
> - error = regmap_update_bits(pwrkey->regmap,
> -pwrkey->baseaddr + PON_PULL_CTL,
> -pwrkey->data->pull_up_bit,
> -pull_up ? pwrkey->data->pull_up_bit : 0);
> - if (error) {
> - dev_err(>dev, "failed to set pull: %d\n", error);
> - return error;
> + if (pwrkey->data->pull_up_bit) {
> + error = regmap_update_bits(pwrkey->regmap,
> +pwrkey->baseaddr + PON_PULL_CTL,
> +pwrkey->data->pull_up_bit,
> +pull_up ? pwrkey->data->pull_up_bit :
> +  0);
> + if (error) {
> + dev_err(>dev, "failed to set pull: %d\n", error);
> + return error;
> + }
>   }
>  
>   error = devm_request

Re: [PATCH v4 0/7] cpufreq-qcom-hw: Implement full OSM programming

2021-04-12 Thread Bjorn Andersson
On Mon 12 Apr 00:11 CDT 2021, Viresh Kumar wrote:

> On 19-01-21, 18:45, AngeloGioacchino Del Regno wrote:
> >   **
> >   ** NOTE: To "view the full picture", please look at the following
> >   ** patch series:
> >   ** https://patchwork.kernel.org/project/linux-arm-msm/list/?series=413355
> >   **  This is a subset of that series.
> >   **
> > 
> > Changes in v4:
> > - Huge patch series has been split for better reviewability,
> >   as suggested by Bjorn
> > - Rebased code on top of 266991721c15 ("cpufreq: qcom-hw: enable boost
> >   support")
> 
> Bjorn, what am I supposed to do with patchset ? Is there anyone who can give
> this some reviews from qcom team ?
> 

Sorry Viresh, I've been postponing reviewing this a few times too many.
I'll take the time in the coming days to look through it properly,
including Taniya's feedback.

Regards,
Bjorn


Re: New 'make dtbs_check W=1' warnings

2021-04-12 Thread Bjorn Andersson
On Mon 12 Apr 13:52 CDT 2021, Arnd Bergmann wrote:

> On Mon, Apr 12, 2021 at 6:01 PM Bjorn Andersson
>  wrote:
> > On Mon 12 Apr 08:14 CDT 2021, Arnd Bergmann wrote:
> > > On Mon, Apr 12, 2021 at 1:32 PM Geert Uytterhoeven  
> > > wrote:
> > > > On Thu, Apr 8, 2021 at 5:08 PM Arnd Bergmann  wrote:
> >
> > So the same binding patch is picked up both in the driver and soc tree?
> > I was expecting that to cause (harmless) conflicts when things arrive in
> > Linus' merge queue?
> >
> > Or are you saying people go the length to create immutable branches for
> > each binding?
> 
> I think it's usually one immutable branch for all the bindings of a given
> merge window. This avoids the merge conflicts, and you can add further
> bindings on the same branch before sending it off to the soc tree.
> 

That would be convenient to have, but the binding changes we depend on
in a given window (in particular if dtbs_check is expected to pass) is
scattered over a wide range of maintainer trees.

> > I'm curious because it's fairly often that we introduce clocks,
> > interconnects etc where the macros from the dt bindings includes aren't
> > available for another release (so we use numerical constants and then go
> > back and fix them up later).
> 
> Ah right, it is particularly bad for platforms that don't have a regular
> layout in these blocks and need to define a new constant every time
> another clock/reset/pin/... is discovered in the downstream sources.
> 

Even blocks following some standardized layout has this problem, because
each platform will have it's own (often similar) set of
clocks/resets/pins.

And going back to dtbs_check, you will continue to see the warnings
about missing compatibles, because most of the case they won't end up in
the soc tree.

> I was mainly referring to the simpler problem of defining a binding
> document for a device once, and then merging the nodes.
> 

I'm sure we all love the hardware that's simple to translate to a DT
binding, unfortunately though we're dealing with complex SoCs.

Regards,
Bjorn


Re: [PATCH V1 1/2] soc: qcom: aoss: Expose send for generic usecase

2021-04-12 Thread Bjorn Andersson
On Fri 09 Apr 02:31 CDT 2021, Manivannan Sadhasivam wrote:

> On Sun, Apr 04, 2021 at 12:17:52PM -0500, Bjorn Andersson wrote:
> > On Fri 02 Apr 01:17 CDT 2021, Deepak Kumar Singh wrote:
> > 
> > > Not all upcoming usecases will have an interface to allow the aoss
> > > driver to hook onto. Expose the send api and create a get function to
> > > enable drivers to send their own messages to aoss.
> > > 
> > > Signed-off-by: Chris Lew 
> > > Signed-off-by: Deepak Kumar Singh 
> > > ---
> > >  drivers/soc/qcom/qcom_aoss.c | 36 +++-
> > >  1 file changed, 35 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/soc/qcom/qcom_aoss.c b/drivers/soc/qcom/qcom_aoss.c
> > > index 53acb94..5c643f0 100644
> > > --- a/drivers/soc/qcom/qcom_aoss.c
> > > +++ b/drivers/soc/qcom/qcom_aoss.c
> > > @@ -8,10 +8,12 @@
> 
> [...]
> 
> > > + pdev = of_find_device_by_node(np);
> > 
> > of_find_device_by_node() will increment the refcount of the underlying
> > struct device of pdev, so you need to platform_device_put() once you're
> > done with it.
> > 
> > As a side effect of not putting the struct device, the devm_kzalloc'ed
> > qmp pointer will remain valid. So care is needed to make sure that the
> > client doesn't end up with a dangling pointer if the qmp device is
> > removed.
> > 
> > My suggestion is that you add a "qmp_put()" function, which invokes
> > platform_device_put() and that you add some sort of tracking ("bool
> > orphan"?) to the struct qmp and make qmp_send() fail if this is set.
> > 
> 
> I think this is a duplication of what the struct device offers. Why
> can't we use the generic infrastructure for this usecase?
> 
> Like using device_initialize() in qmp_probe() along with a release
> callback for "struct device", then using get_device() in qmp_get().
> Then there should also be a qmp_put() API which calls put_device() to
> decrease the refcount.
> 
> Ideally, the final refcount should be dropped in qmp_remove() and then
> the release callback will be called automatically to free "struct qmp".
> 
> > That way if someone unbinds the aoss device, the client will still have
> > a "valid" pointer, but won't be able to qmp_send() after qmp_close() has
> > been called in the aoss remove function.
> > 
> 
> How can someone remove the qmp device if a client is holding its reference?
> 

The device could be unbound using sysfs, in which case remove() is
called and I assumed that devres wouldn't be released until the struct
device's refcount hit 0.

Apparently this does not seems to be how it works, following the unbind
path I see that devres is shot down regardless of the struct device's
refcount.

So we would need to ensure that struct qmp is refcounted on its own.
For this we don't need a separate struct device, we can simply add a
kref to the struct and avoid using devres to keep track of its lifetime.

Regards,
Bjorn

> Thanks,
> Mani
> 
> > Regards,
> > Bjorn
> > 
> > > + if (!pdev)
> > > + return ERR_PTR(-EINVAL);
> > > +
> > > + qmp = platform_get_drvdata(pdev);
> > > + return qmp ? qmp : ERR_PTR(-EPROBE_DEFER);
> > > +}
> > > +EXPORT_SYMBOL(qmp_get);
> > > +
> > >  static int qmp_probe(struct platform_device *pdev)
> > >  {
> > >   struct resource *res;
> > > -- 
> > > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> > > a Linux Foundation Collaborative Project
> > > 


Re: [PATCH] phy: qcom-qmp: remove redundant error of clock bulk

2021-04-12 Thread Bjorn Andersson
On Thu 08 Apr 22:07 CDT 2021, Chunfeng Yun wrote:

> There is error log in clk_bulk_prepare/enable()
> 

Reviewed-by: Bjorn Andersson 

> Signed-off-by: Chunfeng Yun 
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp.c | 8 ++--
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c 
> b/drivers/phy/qualcomm/phy-qcom-qmp.c
> index 9cdebe7..f14b8be 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
> @@ -3598,10 +3598,8 @@ static int qcom_qmp_phy_com_init(struct qmp_phy *qphy)
>   }
>  
>   ret = clk_bulk_prepare_enable(cfg->num_clks, qmp->clks);
> - if (ret) {
> - dev_err(qmp->dev, "failed to enable clks, err=%d\n", ret);
> + if (ret)
>   goto err_rst;
> - }
>  
>   if (cfg->has_phy_dp_com_ctrl) {
>   qphy_setbits(dp_com, QPHY_V3_DP_COM_POWER_DOWN_CTRL,
> @@ -4035,10 +4033,8 @@ static int __maybe_unused 
> qcom_qmp_phy_runtime_resume(struct device *dev)
>   }
>  
>   ret = clk_bulk_prepare_enable(cfg->num_clks, qmp->clks);
> - if (ret) {
> - dev_err(qmp->dev, "failed to enable clks, err=%d\n", ret);
> + if (ret)
>   return ret;
> - }
>  
>   ret = clk_prepare_enable(qphy->pipe_clk);
>   if (ret) {
> -- 
> 1.8.1.1.dirty
> 


Re: [PATCH v7 2/5] soc: qcom: Add SoC sleep stats driver

2021-04-12 Thread Bjorn Andersson
On Tue 06 Apr 05:27 CDT 2021, Maulik Shah wrote:

> From: Mahesh Sivasubramanian 
> 
> Let's add a driver to read the stats from remote processor and
> export to debugfs.
> 
> The driver creates "qcom_sleep_stats" directory in debugfs and
> adds files for various low power mode available. Below is sample
> output with command
> 
> cat /sys/kernel/debug/qcom_sleep_stats/ddr
> count = 0
> Last Entered At = 0
> Last Exited At = 0
> Accumulated Duration = 0
> 
> Signed-off-by: Mahesh Sivasubramanian 
> Signed-off-by: Lina Iyer 
> [mkshah: add subsystem sleep stats, create one file for each stat]
> Signed-off-by: Maulik Shah 

Thanks for picking this up again Maulik, I hope we can get it merged
soon.

> ---
>  drivers/soc/qcom/Kconfig   |  10 ++
>  drivers/soc/qcom/Makefile  |   1 +
>  drivers/soc/qcom/soc_sleep_stats.c | 281 
> +
>  3 files changed, 292 insertions(+)
>  create mode 100644 drivers/soc/qcom/soc_sleep_stats.c
> 
> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index 79b568f..e80b63a 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -190,6 +190,16 @@ config QCOM_SOCINFO
>Say yes here to support the Qualcomm socinfo driver, providing
>information about the SoC to user space.
>  
> +config QCOM_SOC_SLEEP_STATS
> + tristate "Qualcomm Technologies, Inc. (QTI) SoC sleep stats driver"
> + depends on ARCH_QCOM && DEBUG_FS || COMPILE_TEST
> + depends on QCOM_SMEM
> + help
> +   Qualcomm Technologies, Inc. (QTI) SoC sleep stats driver to read
> +   the shared memory exported by the remote processor related to
> +   various SoC level low power modes statistics and export to debugfs
> +   interface.
> +
>  config QCOM_WCNSS_CTRL
>   tristate "Qualcomm WCNSS control driver"
>   depends on ARCH_QCOM || COMPILE_TEST
> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
> index ad675a6..5f30d74 100644
> --- a/drivers/soc/qcom/Makefile
> +++ b/drivers/soc/qcom/Makefile
> @@ -20,6 +20,7 @@ obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
>  obj-$(CONFIG_QCOM_SMP2P) += smp2p.o
>  obj-$(CONFIG_QCOM_SMSM)  += smsm.o
>  obj-$(CONFIG_QCOM_SOCINFO)   += socinfo.o
> +obj-$(CONFIG_QCOM_SOC_SLEEP_STATS)   += soc_sleep_stats.o
>  obj-$(CONFIG_QCOM_WCNSS_CTRL) += wcnss_ctrl.o
>  obj-$(CONFIG_QCOM_APR) += apr.o
>  obj-$(CONFIG_QCOM_LLCC) += llcc-qcom.o
> diff --git a/drivers/soc/qcom/soc_sleep_stats.c 
> b/drivers/soc/qcom/soc_sleep_stats.c
> new file mode 100644
> index 000..6dfc239
> --- /dev/null
> +++ b/drivers/soc/qcom/soc_sleep_stats.c
> @@ -0,0 +1,281 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2011-2021, The Linux Foundation. All rights reserved.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +
> +#define STAT_TYPE_ADDR   0x0
> +#define COUNT_ADDR   0x4
> +#define LAST_ENTERED_AT_ADDR 0x8
> +#define LAST_EXITED_AT_ADDR  0x10
> +#define ACCUMULATED_ADDR 0x18
> +#define CLIENT_VOTES_ADDR0x1c

These aren't "addresses", they are offsets, and they are actually just
offset_of() on 

> +
> +#define STAT_OFFSET(record_no, type) (((record_no)*(sizeof(struct 
> sleep_stats))) + (type))
> +#define APPENDED_STAT_OFFSET(record_no) ((record_no)*(sizeof(struct 
> appended_stats)))
> +
> +#ifndef readq
> +#define readq(a) ({  \
> + u64 val = readl((a) + 4);   \
> + val <<= 32; \
> + val |=  readl((a)); \
> + val;\
> +})

I don't like this macro, can you please write it as a static inline,
with a comment that the operation is not expected to be atomic?

> +#endif
> +
> +struct subsystem_data {
> + const char *name;
> + u32 smem_item;
> + u32 pid;
> +};
> +
> +static const struct subsystem_data subsystems[] = {
> + { "modem", 605, 1 },
> + { "wpss", 605, 13 },
> + { "adsp", 606, 2 },
> + { "cdsp", 607, 5 },
> + { "slpi", 608, 3 },
> + { "gpu", 609, 0 },
> + { "display", 610, 0 },
> + { "adsp_island", 613, 2 },
> + { "slpi_island", 613, 3 },
> +};
> +
> +struct stats_config {
> + u32 offset_addr;
> + u32 num_records;

Neither of these must be 32-bit, so please use int...or size_t instead.

> + bool appended_stats_avail;
> +};
> +
> +struct stats_prv_data {

I don't see any "public" data, so how about just dropping "prv_"?

> + bool appended_stats_avail;
> + void __iomem *reg;

Call this "base" instead.

> +};
> +
> +struct sleep_stats {
> + u32 stat_type;
> + u32 count;
> + u64 last_entered_at;
> + u64 last_exited_at;
> + u64 accumulated;
> +};
> +
> +struct appended_stats {
> + u32 client_votes;
> + u32 reserved[3];
> +};
> +
> +static void print_sleep_stats(struct seq_file *s, const struct sleep_stats 
> *stat)

I would like 

Re: New 'make dtbs_check W=1' warnings

2021-04-12 Thread Bjorn Andersson
On Mon 12 Apr 08:14 CDT 2021, Arnd Bergmann wrote:

> On Mon, Apr 12, 2021 at 1:32 PM Geert Uytterhoeven  
> wrote:
> > On Thu, Apr 8, 2021 at 5:08 PM Arnd Bergmann  wrote:
> 
> > >
> > > For this merge window, I don't think any of them are show-stoppers (Rob, 
> > > let me
> > > know if you disagree), but in the long run we may want to gradually 
> > > enforce
> > > a rule about not merging changes that introduce any new warnings, in 
> > > order to
> > > have a chance of cleaning up the existing ones.
> >
> > This may not be as simple as it sounds, as DT binding updates typically
> > follow a different path than DTS(i) updates.  DT bindings updates may be
> > picked up by a subsystem maintainer, by Rob, or by the platform
> > maintainer.
> 
> I checked out the bindings from linux-next, which seems to have covered
> most of these. Sometimes it pays off to be lazy and merge them after
> everyone else.
> 
> > For trivial updates (e.g. adding a compatible value, and sometimes
> > extending a limit), a DTS(i) update may be accepted by the platform
> > maintainer before the corresponding DT binding update.  The latter may
> > even be merged one or more kernel revisions later, especially when
> > involving subsystems that are not traditionally rooted into platforms
> > using DT.
> >
> > Of course we could mention any expected warning regressions in our pull
> > requests for soc.
> 
> Right, that would certainly help. Some maintainers send all binding
> updates both to the driver maintainers and to the soc tree, along
> with the other changes that only go into one tree. That is of course
> also more work on your side, but it solves the problem entirely.
> 

So the same binding patch is picked up both in the driver and soc tree?
I was expecting that to cause (harmless) conflicts when things arrive in
Linus' merge queue?

Or are you saying people go the length to create immutable branches for
each binding?


I'm curious because it's fairly often that we introduce clocks,
interconnects etc where the macros from the dt bindings includes aren't
available for another release (so we use numerical constants and then go
back and fix them up later).

Regards,
Bjorn

> > > renesas/r8a774a1-beacon-rzg2m-kit.dt.yaml: csi2@feaa: ports:
> > > 'port@0' is a required property
> >
> > [...]
> >
> > I've replied to these as a response to your PR reply, see
> > https://lore.kernel.org/linux-renesas-soc/camuhmdwhlnxgbsjp7vkudx-ynr9rskfke5ge5q_taru6hp9...@mail.gmail.com/
> 
> Ok, thanks.
> 
>   Arnd


Re: [PATCH v4] media: venus : hfi: add venus image info into smem

2021-04-09 Thread Bjorn Andersson
On Fri 09 Apr 06:46 CDT 2021, Dikshita Agarwal wrote:

> Fill fw version info into smem to be printed as part of
> soc info.
> 
> Signed-off-by: Dikshita Agarwal 
> Reported-by: kernel test robot 

Why/how did kernel test robot tell you that you should write the
firmware version in SMEM?

> 
> change since v3:
>  added dependency on QCOM_SMEM (Stephen)
> ---
>  drivers/media/platform/Kconfig   |  2 +-
>  drivers/media/platform/qcom/venus/hfi_msgs.c | 21 +++--
>  2 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index fd1831e..9c75e88 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -543,7 +543,7 @@ config VIDEO_TI_VPE_DEBUG
>  
>  config VIDEO_QCOM_VENUS
>   tristate "Qualcomm Venus V4L2 encoder/decoder driver"
> - depends on VIDEO_DEV && VIDEO_V4L2
> + depends on VIDEO_DEV && VIDEO_V4L2 && QCOM_SMEM
>   depends on (ARCH_QCOM && IOMMU_DMA) || COMPILE_TEST
>   select QCOM_MDT_LOADER if ARCH_QCOM
>   select QCOM_SCM if ARCH_QCOM
> diff --git a/drivers/media/platform/qcom/venus/hfi_msgs.c 
> b/drivers/media/platform/qcom/venus/hfi_msgs.c
> index 06a1908..74cfc4f 100644
> --- a/drivers/media/platform/qcom/venus/hfi_msgs.c
> +++ b/drivers/media/platform/qcom/venus/hfi_msgs.c
> @@ -6,6 +6,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #include "core.h"
> @@ -14,6 +15,10 @@
>  #include "hfi_msgs.h"
>  #include "hfi_parser.h"
>  
> +#define SMEM_IMG_VER_TBL 469
> +#define VER_STR_SZ   128
> +#define SMEM_IMG_OFFSET_VENUS (14 * 128)
> +
>  static void event_seq_changed(struct venus_core *core, struct venus_inst 
> *inst,
> struct hfi_msg_event_notify_pkt *pkt)
>  {
> @@ -239,15 +244,27 @@ static void
>  sys_get_prop_image_version(struct device *dev,
>  struct hfi_msg_sys_property_info_pkt *pkt)
>  {
> + u8 *smem_tbl_ptr;
> + u8 *img_ver;
>   int req_bytes;
> + size_t smem_blk_sz;
>  
>   req_bytes = pkt->hdr.size - sizeof(*pkt);
>  
> - if (req_bytes < 128 || !pkt->data[1] || pkt->num_properties > 1)
> + if (req_bytes < VER_STR_SZ || !pkt->data[1] || pkt->num_properties > 1)
>   /* bad packet */
>   return;
>  
> - dev_dbg(dev, VDBGL "F/W version: %s\n", (u8 *)>data[1]);
> + img_ver = (u8 *)>data[1];
> +
> + dev_dbg(dev, VDBGL "F/W version: %s\n", img_ver);
> +
> + smem_tbl_ptr = qcom_smem_get(QCOM_SMEM_HOST_ANY,
> + SMEM_IMG_VER_TBL, _blk_sz);
> + if (smem_tbl_ptr &&
> + smem_blk_sz >= SMEM_IMG_OFFSET_VENUS + VER_STR_SZ)
> + memcpy(smem_tbl_ptr + SMEM_IMG_OFFSET_VENUS,
> +img_ver, VER_STR_SZ);

I think you should unwrap lines, the conditional isn't even more than 80
chars...

Apart from these nits I think the patch looks good.

Regards,
Bjorn

>  }
>  
>  static void hfi_sys_property_info(struct venus_core *core,
> -- 
> 2.7.4
> 


[GIT PULL] remoteproc fixes for v5.12

2021-04-09 Thread Bjorn Andersson
The following changes since commit a38fd8748464831584a19438cbb3082b5a2dab15:

  Linux 5.12-rc2 (2021-03-05 17:33:41 -0800)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/andersson/remoteproc.git 
tags/rproc-v5.12-fixes

for you to fetch changes up to 9afeefcf06fc7b4bdab06a6e2cb06745bded34dd:

  remoteproc: pru: Fix firmware loading crashes on K3 SoCs (2021-03-17 14:15:07 
-0500)


remoteproc fixes for v5.12

This fixes an issue with firmware loading on the TI K3 PRU, fixes
compatibility with GNU binutils for the same and resolves link error
due to a 64-bit division in the Qualcomm PIL info.

It also recognizes Mathieu Poirier as co-maintainer of the remoteproc
and rpmsg subsystems.


Arnd Bergmann (1):
  remoteproc: qcom: pil_info: avoid 64-bit division

Dimitar Dimitrov (1):
  remoteproc: pru: Fix loading of GNU Binutils ELF

Mathieu Poirier (1):
  MAINTAINERS: Add co-maintainer for remoteproc/RPMSG subsystems

Suman Anna (1):
  remoteproc: pru: Fix firmware loading crashes on K3 SoCs

 MAINTAINERS|  2 ++
 drivers/remoteproc/pru_rproc.c | 20 +++-
 drivers/remoteproc/qcom_pil_info.c |  2 +-
 3 files changed, 22 insertions(+), 2 deletions(-)


Re: [PATCH 1/3] dt-bindings: mfd: pm8008: Add IRQ listing

2021-04-09 Thread Bjorn Andersson
On Thu 08 Apr 19:38 CDT 2021, Guru Das Srinagesh wrote:

> Add a header file listing all of the IRQs that Qualcomm Technologies,
> Inc. PM8008 supports. The constants defined in this file may be used in
> the client device tree node to specify interrupts.
> 
> Change-Id: I13fb096da54458f2882e8d853a3ad9c379e7d5a9

Please remember to drop the Change-Id when posting to the mailing lists.


We typically don't have defines for the IRQ numbers, but I don't mind.
Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> Signed-off-by: Guru Das Srinagesh 
> ---
>  include/dt-bindings/mfd/qcom-pm8008.h | 19 +++
>  1 file changed, 19 insertions(+)
>  create mode 100644 include/dt-bindings/mfd/qcom-pm8008.h
> 
> diff --git a/include/dt-bindings/mfd/qcom-pm8008.h 
> b/include/dt-bindings/mfd/qcom-pm8008.h
> new file mode 100644
> index 000..eca9448
> --- /dev/null
> +++ b/include/dt-bindings/mfd/qcom-pm8008.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2021 The Linux Foundation. All rights reserved.
> + */
> +
> +#ifndef __DT_BINDINGS_MFD_QCOM_PM8008_H
> +#define __DT_BINDINGS_MFD_QCOM_PM8008_H
> +
> +/* PM8008 IRQ numbers */
> +#define PM8008_IRQ_MISC_UVLO 0
> +#define PM8008_IRQ_MISC_OVLO 1
> +#define PM8008_IRQ_MISC_OTST22
> +#define PM8008_IRQ_MISC_OTST33
> +#define PM8008_IRQ_MISC_LDO_OCP  4
> +#define PM8008_IRQ_TEMP_ALARM5
> +#define PM8008_IRQ_GPIO1 6
> +#define PM8008_IRQ_GPIO2 7
> +
> +#endif
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 


Re: [PATCH V2 4/4] dt-bindings: rtc: qcom-pm8xxx-rtc: Add qcom pm8xxx rtc bindings

2021-04-09 Thread Bjorn Andersson
On Fri 09 Apr 08:59 CDT 2021, satya priya wrote:

> Add binding doc for qcom pm8xxx rtc device.
> 

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> Signed-off-by: satya priya 
> ---
> Changes in V2:
>  - Added this in V2 to have separate binding for rtc node.
> 
>  .../devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml   | 62 
> ++
>  1 file changed, 62 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml 
> b/Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml
> new file mode 100644
> index 000..4fba6db
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/rtc/qcom-pm8xxx-rtc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm PM8xxx PMIC RTC device
> +
> +maintainers:
> +  - Satya Priya 
> +
> +properties:
> +  compatible:
> +enum:
> +  - qcom,pm8058-rtc
> +  - qcom,pm8921-rtc
> +  - qcom,pm8941-rtc
> +  - qcom,pm8018-rtc
> +  - qcom,pmk8350-rtc
> +
> +  reg:
> +maxItems: 1
> +
> +  interrupts:
> +maxItems: 1
> +
> +  allow-set-time:
> +$ref: /schemas/types.yaml#/definitions/flag
> +description:
> +  Indicates that the setting of RTC time is allowed by the host CPU.
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +#include 
> +spmi_bus: spmi@c44 {
> +  reg = <0x0c44 0x1100>;
> +  #address-cells = <2>;
> +  #size-cells = <0>;
> +  pmicintc: pmic@0 {
> +reg = <0x0 SPMI_USID>;
> +compatible = "qcom,pm8921";
> +interrupts = <104 8>;
> +#interrupt-cells = <2>;
> +interrupt-controller;
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +pm8921_rtc: rtc@11d {
> +  compatible = "qcom,pm8921-rtc";
> +  reg = <0x11d>;
> +  interrupts = <0x27 0>;
> +};
> +  };
> +};
> +...
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member 
> of Code Aurora Forum, hosted by The Linux Foundation
> 


Re: [PATCH V2 3/4] dt-bindings: mfd: Convert pm8xxx bindings to yaml

2021-04-09 Thread Bjorn Andersson
,0 +1,54 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/qcom-pm8xxx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm PM8xxx PMIC multi-function devices
> +
> +maintainers:
> +  - Satya Priya 
> +
> +description: |
> +  The PM8xxx family of Power Management ICs are used to provide regulated
> +  voltages and other various functionality to Qualcomm SoCs.
> +
> +properties:
> +  compatible:
> +enum:
> +  - qcom,pm8058
> +  - qcom,pm8821
> +  - qcom,pm8921
> +
> +  reg:
> +maxItems: 1
> +
> +  '#address-cells':
> +const: 1
> +
> +  '#size-cells':
> +const: 0
> +
> +  interrupts:
> +maxItems: 1
> +
> +  '#interrupt-cells':
> +const: 2
> +
> +  interrupt-controller: true
> +
> +patternProperties:
> +  "rtc@[0-9a-f]+$":
> +type: object
> +$ref: "../rtc/qcom-pm8xxx-rtc.yaml"

This doesn't exist, so patch 3 and 4 should come in opposite order...

Apart from tat I think this looks good.

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> +
> +required:
> +  - compatible
> +  - '#address-cells'
> +  - '#size-cells'
> +  - interrupts
> +  - '#interrupt-cells'
> +  - interrupt-controller
> +
> +additionalProperties: false
> +...
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member 
> of Code Aurora Forum, hosted by The Linux Foundation
> 


Re: [PATCH RESEND v8 7/7] arm64: dts: qcom: Harmonize DWC USB3 DT nodes name

2021-04-09 Thread Bjorn Andersson
On Fri 09 Apr 06:30 CDT 2021, Serge Semin wrote:

> In accordance with the DWC USB3 bindings the corresponding node
> name is suppose to comply with the Generic USB HCD DT schema, which
> requires the USB nodes to have the name acceptable by the regexp:
> "^usb(@.*)?" . Make sure the "snps,dwc3"-compatible nodes are correctly
> named.
> 
> Signed-off-by: Serge Semin 
> Acked-by: Krzysztof Kozlowski 
> Reviewed-by: Bjorn Andersson 

As mentioned previously, I would like to merge this through the qcom soc
tree to avoid conflicts with other activities, but need the driver code
(patch 6) to land first.

Regards,
Bjorn

> ---
>  arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi | 4 ++--
>  arch/arm64/boot/dts/qcom/ipq8074.dtsi| 4 ++--
>  arch/arm64/boot/dts/qcom/msm8996.dtsi| 4 ++--
>  arch/arm64/boot/dts/qcom/msm8998.dtsi| 2 +-
>  arch/arm64/boot/dts/qcom/qcs404-evb.dtsi | 2 +-
>  arch/arm64/boot/dts/qcom/qcs404.dtsi | 4 ++--
>  arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +-
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 4 ++--
>  arch/arm64/boot/dts/qcom/sm8150.dtsi | 2 +-
>  9 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi 
> b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> index defcbd15edf9..34e97da98270 100644
> --- a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> +++ b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> @@ -1064,7 +1064,7 @@  {
>   status = "okay";
>   extcon = <_id>;
>  
> - dwc3@760 {
> + usb@760 {
>   extcon = <_id>;
>   dr_mode = "otg";
>   maximum-speed = "high-speed";
> @@ -1075,7 +1075,7 @@  {
>   status = "okay";
>   extcon = <_id>;
>  
> - dwc3@6a0 {
> + usb@6a0 {
>   extcon = <_id>;
>   dr_mode = "otg";
>   };
> diff --git a/arch/arm64/boot/dts/qcom/ipq8074.dtsi 
> b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
> index a32e5e79ab0b..7df4eb710aae 100644
> --- a/arch/arm64/boot/dts/qcom/ipq8074.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
> @@ -427,7 +427,7 @@ usb_0: usb@8af8800 {
>   resets = < GCC_USB0_BCR>;
>   status = "disabled";
>  
> - dwc_0: dwc3@8a0 {
> + dwc_0: usb@8a0 {
>   compatible = "snps,dwc3";
>   reg = <0x8a0 0xcd00>;
>   interrupts = ;
> @@ -468,7 +468,7 @@ usb_1: usb@8cf8800 {
>   resets = < GCC_USB1_BCR>;
>   status = "disabled";
>  
> - dwc_1: dwc3@8c0 {
> + dwc_1: usb@8c0 {
>   compatible = "snps,dwc3";
>   reg = <0x8c0 0xcd00>;
>   interrupts = ;
> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi 
> b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> index ce430ba9c118..9eb31b3e6ee7 100644
> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> @@ -1772,7 +1772,7 @@ usb3: usb@6af8800 {
>   power-domains = < USB30_GDSC>;
>   status = "disabled";
>  
> - dwc3@6a0 {
> + usb@6a0 {
>   compatible = "snps,dwc3";
>   reg = <0x06a0 0xcc00>;
>   interrupts = <0 131 IRQ_TYPE_LEVEL_HIGH>;
> @@ -1983,7 +1983,7 @@ usb2: usb@76f8800 {
>   power-domains = < USB30_GDSC>;
>   status = "disabled";
>  
> - dwc3@760 {
> + usb@760 {
>   compatible = "snps,dwc3";
>   reg = <0x0760 0xcc00>;
>   interrupts = <0 138 IRQ_TYPE_LEVEL_HIGH>;
> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi 
> b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> index 1f2e93aa6553..9141c5d09b59 100644
> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> @@ -1962,7 +1962,7 @@ usb3: usb@a8f8800 {
>  
>   resets = < GCC_USB_30_BCR>;
>  
> - usb3_dwc3: dwc3@a80 {
> + usb3_dwc3: usb@a80 {
>   compatible = &quo

Re: [PATCH RESEND v8 6/7] usb: dwc3: qcom: Detect DWC3 DT-nodes using compatible string

2021-04-09 Thread Bjorn Andersson
On Fri 09 Apr 06:30 CDT 2021, Serge Semin wrote:

> In accordance with the USB HCD/DRD schema all the USB controllers are
> supposed to have DT-nodes named with prefix "^usb(@.*)?". Since the
> existing DT-nodes will be renamed in a subsequent patch let's fix the DWC3
> Qcom-specific code to detect the DWC3 sub-node just by checking its
> compatible string to match the "snps,dwc3". The semantic of the code
> won't change seeing all the DWC USB3 nodes are supposed to have the
> compatible property with any of those strings set.
> 

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> Signed-off-by: Serge Semin 
> 
> ---
> 
> Changelog v7:
> - Replace "of_get_child_by_name(np, "usb") ?: of_get_child_by_name(np, 
> "dwc3");"
>   pattern with using of_get_compatible_child() method.
> - Discard Bjorn Andersson Reviewed-by tag since the patch content
>   has been changed.
> ---
>  drivers/usb/dwc3/dwc3-qcom.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index fcaf04483ad0..617a1be88371 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -644,7 +644,7 @@ static int dwc3_qcom_of_register_core(struct 
> platform_device *pdev)
>   struct device   *dev = >dev;
>   int ret;
>  
> - dwc3_np = of_get_child_by_name(np, "dwc3");
> + dwc3_np = of_get_compatible_child(np, "snps,dwc3");
>   if (!dwc3_np) {
>   dev_err(dev, "failed to find dwc3 core child\n");
>   return -ENODEV;
> -- 
> 2.30.1
> 


Re: [PATCH 2/2] pinctrl: qcom-pmic-gpio: Add support for pm8008

2021-04-07 Thread Bjorn Andersson
On Wed 07 Apr 17:35 CDT 2021, Guru Das Srinagesh wrote:

> Add support for the two GPIOs present on PM8008.
> 
> Signed-off-by: Guru Das Srinagesh 
> ---
>  drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c 
> b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> index c2b9f2e..76e997a 100644
> --- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> +++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> @@ -1137,6 +1137,7 @@ static const struct of_device_id pmic_gpio_of_match[] = 
> {
>   { .compatible = "qcom,pm6150l-gpio", .data = (void *) 12 },
>   /* pmx55 has 11 GPIOs with holes on 3, 7, 10, 11 */
>   { .compatible = "qcom,pmx55-gpio", .data = (void *) 11 },
> + { .compatible = "qcom,pm8008-gpio", .data = (void *) 2 },

As with the binding, please keep these sorted alphabetically.

With that:

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

>   { },
>  };
>  
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 


Re: [PATCH 1/2] dt-bindings: pinctrl: qcom-pmic-gpio: Add pm8008 support

2021-04-07 Thread Bjorn Andersson
On Wed 07 Apr 17:34 CDT 2021, Guru Das Srinagesh wrote:

> Add support for the PM8008 GPIO support to the Qualcomm PMIC GPIO
> binding.
> 
> Signed-off-by: Guru Das Srinagesh 
> ---
>  Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.txt 
> b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.txt
> index 70e119b..1818481 100644
> --- a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.txt
> @@ -36,6 +36,7 @@ PMIC's from Qualcomm.
>   "qcom,pm6150-gpio"
>   "qcom,pm6150l-gpio"
>   "qcom,pmx55-gpio"
> + "qcom,pm8008-gpio"

Please keep these sorted alphabetically (i.e. '8' < 'x')

With that

Acked-by: Bjorn Andersson 

Regards,
Bjorn

>  
>   And must contain either "qcom,spmi-gpio" or "qcom,ssbi-gpio"
>   if the device is on an spmi bus or an ssbi bus respectively
> @@ -125,6 +126,7 @@ to specify in a pin configuration subnode:
>   gpio1-gpio12 for pm6150l
>   gpio1-gpio11 for pmx55 (holes on gpio3, gpio7, gpio10
>   and gpio11)
> + gpio1-gpio2 for pm8008
>  
>  - function:
>   Usage: required
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 


Re: [PATCH 2/3] dt-bindings: mfd: Convert pm8xxx bindings to yaml

2021-04-07 Thread Bjorn Andersson
On Wed 07 Apr 10:37 CDT 2021, ska...@codeaurora.org wrote:

> Hi Bjorn,
> 
> On 2021-03-11 22:33, Bjorn Andersson wrote:
> > On Thu 11 Mar 01:29 CST 2021, satya priya wrote:
[..]
> > > +patternProperties:
> > > +  "rtc@[0-9a-f]+$":
> > 
> > Can we somehow link this to individual binding docs instead of listing
> > all the possible functions here?
> > 
> 
> you mean we should split this into two:
> qcom-pm8xxx.yaml and qcom-pm8xxx-rtc.yaml
> Please correct me if wrong.
> 

Right, I'm worried that it will be quite hard to maintain this document
once we start adding all the various pmic blocks to it. So if we somehow
can maintain a series of qcom-pm8xxx-.yaml and just ref them into
the main PMIC definition.

@Rob, can you give us some guidance on how to structure this binding,
with the various PMICs described will have some defined subset of a
larger set of hardware blocks that's often shared between versions?

Regards,
Bjorn


Re: [PATCH v1 1/1] kernel.h: Split out panic and oops helpers

2021-04-06 Thread Bjorn Andersson
On Tue 06 Apr 08:31 CDT 2021, Andy Shevchenko wrote:

> kernel.h is being used as a dump for all kinds of stuff for a long time.
> Here is the attempt to start cleaning it up by splitting out panic and
> oops helpers.
> 
> At the same time convert users in header and lib folder to use new header.
> Though for time being include new header back to kernel.h to avoid twisted
> indirected includes for existing users.
> 

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn


Re: [PATCH 3/7] crypto: qce: Add mode for rfc4309

2021-04-05 Thread Bjorn Andersson
On Thu 25 Feb 12:27 CST 2021, Thara Gopinath wrote:

> rf4309 is the specification that uses aes ccm algorithms with IPsec
> security packets. Add a submode to identify rfc4309 ccm(aes) algorithm
> in the crypto driver.
> 
> Signed-off-by: Thara Gopinath 
> ---
>  drivers/crypto/qce/common.h | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/crypto/qce/common.h b/drivers/crypto/qce/common.h
> index 3bc244bcca2d..3ffe719b79e4 100644
> --- a/drivers/crypto/qce/common.h
> +++ b/drivers/crypto/qce/common.h
> @@ -51,9 +51,11 @@
>  #define QCE_MODE_CCM BIT(12)
>  #define QCE_MODE_MASKGENMASK(12, 8)
>  
> +#define QCE_MODE_CCM_RFC4309 BIT(13)
> +
>  /* cipher encryption/decryption operations */
> -#define QCE_ENCRYPT  BIT(13)
> -#define QCE_DECRYPT  BIT(14)
> +#define QCE_ENCRYPT  BIT(14)
> +#define QCE_DECRYPT  BIT(15)

Can't we move these further up, so that next time we want to add
something it doesn't require that we also move the ENC/DEC bits?

>  
>  #define IS_DES(flags)(flags & QCE_ALG_DES)
>  #define IS_3DES(flags)   (flags & QCE_ALG_3DES)
> @@ -73,6 +75,7 @@
>  #define IS_CTR(mode) (mode & QCE_MODE_CTR)
>  #define IS_XTS(mode) (mode & QCE_MODE_XTS)
>  #define IS_CCM(mode) (mode & QCE_MODE_CCM)
> +#define IS_CCM_RFC4309(mode) ((mode) & QCE_MODE_CCM_RFC4309)

While leaving room for the typical macro issues, none of the other
macros wrap the argument in parenthesis. Please follow the style of the
driver, and perhaps follow up with a cleanup patch that just wraps them
all in parenthesis?

Regards,
Bjorn

>  
>  #define IS_ENCRYPT(dir)  (dir & QCE_ENCRYPT)
>  #define IS_DECRYPT(dir)  (dir & QCE_DECRYPT)
> -- 
> 2.25.1
> 


Re: [PATCH 2/7] crypto: qce: common: Make result dump optional

2021-04-05 Thread Bjorn Andersson
On Thu 25 Feb 12:27 CST 2021, Thara Gopinath wrote:

> Qualcomm crypto engine allows for IV registers and status register
> to be concatenated to the output. This option is enabled by setting the
> RESULTS_DUMP field in GOPROC  register. This is useful for most of the
> algorithms to either retrieve status of operation or in case of
> authentication algorithms to retrieve the mac. But for ccm
> algorithms, the mac is part of the output stream and not retrieved
> from the IV registers, thus needing a separate buffer to retrieve it.
> Make enabling RESULTS_DUMP field optional so that algorithms can choose
> whether or not to enable the option.
> Note that in this patch, the enabled algorithms always choose
> RESULTS_DUMP to be enabled. But later with the introduction of ccm
> algorithms, this changes.
> 
> Signed-off-by: Thara Gopinath 

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> ---
>  drivers/crypto/qce/common.c | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/crypto/qce/common.c b/drivers/crypto/qce/common.c
> index 7c3cb483749e..2485aa371d83 100644
> --- a/drivers/crypto/qce/common.c
> +++ b/drivers/crypto/qce/common.c
> @@ -88,9 +88,12 @@ static void qce_setup_config(struct qce_device *qce)
>   qce_write(qce, REG_CONFIG, config);
>  }
>  
> -static inline void qce_crypto_go(struct qce_device *qce)
> +static inline void qce_crypto_go(struct qce_device *qce, bool result_dump)
>  {
> - qce_write(qce, REG_GOPROC, BIT(GO_SHIFT) | BIT(RESULTS_DUMP_SHIFT));
> + if (result_dump)
> + qce_write(qce, REG_GOPROC, BIT(GO_SHIFT) | 
> BIT(RESULTS_DUMP_SHIFT));
> + else
> + qce_write(qce, REG_GOPROC, BIT(GO_SHIFT));
>  }
>  
>  #ifdef CONFIG_CRYPTO_DEV_QCE_SHA
> @@ -219,7 +222,7 @@ static int qce_setup_regs_ahash(struct 
> crypto_async_request *async_req)
>   config = qce_config_reg(qce, 1);
>   qce_write(qce, REG_CONFIG, config);
>  
> - qce_crypto_go(qce);
> + qce_crypto_go(qce, true);
>  
>   return 0;
>  }
> @@ -380,7 +383,7 @@ static int qce_setup_regs_skcipher(struct 
> crypto_async_request *async_req)
>   config = qce_config_reg(qce, 1);
>   qce_write(qce, REG_CONFIG, config);
>  
> - qce_crypto_go(qce);
> + qce_crypto_go(qce, true);
>  
>   return 0;
>  }
> -- 
> 2.25.1
> 


Re: [PATCH 6/7] crypto: qce: common: Add support for AEAD algorithms

2021-04-05 Thread Bjorn Andersson
On Thu 25 Feb 12:27 CST 2021, Thara Gopinath wrote:

> Add register programming sequence for enabling AEAD
> algorithms on the Qualcomm crypto engine.
> 
> Signed-off-by: Thara Gopinath 
> ---
>  drivers/crypto/qce/common.c | 155 +++-
>  1 file changed, 153 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/crypto/qce/common.c b/drivers/crypto/qce/common.c
> index 05a71c5ecf61..54d209cb0525 100644
> --- a/drivers/crypto/qce/common.c
> +++ b/drivers/crypto/qce/common.c
> @@ -15,6 +15,16 @@
>  #include "core.h"
>  #include "regs-v5.h"
>  #include "sha.h"
> +#include "aead.h"
> +
> +static const u32 std_iv_sha1[SHA256_DIGEST_SIZE / sizeof(u32)] = {
> + SHA1_H0, SHA1_H1, SHA1_H2, SHA1_H3, SHA1_H4, 0, 0, 0
> +};
> +
> +static const u32 std_iv_sha256[SHA256_DIGEST_SIZE / sizeof(u32)] = {
> + SHA256_H0, SHA256_H1, SHA256_H2, SHA256_H3,
> + SHA256_H4, SHA256_H5, SHA256_H6, SHA256_H7
> +};
>  
>  static inline u32 qce_read(struct qce_device *qce, u32 offset)
>  {
> @@ -96,7 +106,7 @@ static inline void qce_crypto_go(struct qce_device *qce, 
> bool result_dump)
>   qce_write(qce, REG_GOPROC, BIT(GO_SHIFT));
>  }
>  
> -#ifdef CONFIG_CRYPTO_DEV_QCE_SHA
> +#if defined(CONFIG_CRYPTO_DEV_QCE_SHA) || defined(CONFIG_CRYPTO_DEV_QCE_AEAD)
>  static u32 qce_auth_cfg(unsigned long flags, u32 key_size, u32 auth_size)
>  {
>   u32 cfg = 0;
> @@ -139,7 +149,9 @@ static u32 qce_auth_cfg(unsigned long flags, u32 
> key_size, u32 auth_size)
>  
>   return cfg;
>  }
> +#endif
>  
> +#ifdef CONFIG_CRYPTO_DEV_QCE_SHA
>  static int qce_setup_regs_ahash(struct crypto_async_request *async_req)
>  {
>   struct ahash_request *req = ahash_request_cast(async_req);
> @@ -225,7 +237,7 @@ static int qce_setup_regs_ahash(struct 
> crypto_async_request *async_req)
>  }
>  #endif
>  
> -#ifdef CONFIG_CRYPTO_DEV_QCE_SKCIPHER
> +#if defined(CONFIG_CRYPTO_DEV_QCE_SKCIPHER) || 
> defined(CONFIG_CRYPTO_DEV_QCE_AEAD)
>  static u32 qce_encr_cfg(unsigned long flags, u32 aes_key_size)
>  {
>   u32 cfg = 0;
> @@ -271,7 +283,9 @@ static u32 qce_encr_cfg(unsigned long flags, u32 
> aes_key_size)
>  
>   return cfg;
>  }
> +#endif
>  
> +#ifdef CONFIG_CRYPTO_DEV_QCE_SKCIPHER
>  static void qce_xts_swapiv(__be32 *dst, const u8 *src, unsigned int ivsize)
>  {
>   u8 swap[QCE_AES_IV_LENGTH];
> @@ -386,6 +400,139 @@ static int qce_setup_regs_skcipher(struct 
> crypto_async_request *async_req)
>  }
>  #endif
>  
> +#ifdef CONFIG_CRYPTO_DEV_QCE_AEAD
> +static int qce_setup_regs_aead(struct crypto_async_request *async_req)
> +{
> + struct aead_request *req = aead_request_cast(async_req);
> + struct qce_aead_reqctx *rctx = aead_request_ctx(req);
> + struct qce_aead_ctx *ctx = crypto_tfm_ctx(async_req->tfm);
> + struct qce_alg_template *tmpl = to_aead_tmpl(crypto_aead_reqtfm(req));
> + struct qce_device *qce = tmpl->qce;
> + __be32 enckey[QCE_MAX_CIPHER_KEY_SIZE / sizeof(__be32)] = {0};
> + __be32 enciv[QCE_MAX_IV_SIZE / sizeof(__be32)] = {0};
> + __be32 authkey[QCE_SHA_HMAC_KEY_SIZE / sizeof(__be32)] = {0};
> + __be32 authiv[SHA256_DIGEST_SIZE / sizeof(__be32)] = {0};
> + __be32 authnonce[QCE_MAX_NONCE / sizeof(__be32)] = {0};
> + unsigned int enc_keylen = ctx->enc_keylen;
> + unsigned int auth_keylen = ctx->auth_keylen;
> + unsigned int enc_ivsize = rctx->ivsize;
> + unsigned int auth_ivsize;
> + unsigned int enckey_words, enciv_words;
> + unsigned int authkey_words, authiv_words, authnonce_words;
> + unsigned long flags = rctx->flags;
> + u32 encr_cfg = 0, auth_cfg = 0, config, totallen;

I don't see any reason to initialize encr_cfg or auth_cfg.

> + u32 *iv_last_word;
> +
> + qce_setup_config(qce);
> +
> + /* Write encryption key */
> + qce_cpu_to_be32p_array(enckey, ctx->enc_key, enc_keylen);
> + enckey_words = enc_keylen / sizeof(u32);
> + qce_write_array(qce, REG_ENCR_KEY0, (u32 *)enckey, enckey_words);

Afaict all "array registers" in this function are affected by the
CRYPTO_SETUP little endian bit, but you set this bit before launching
the operation dependent on IS_CCM(). So is this really working for the
!IS_CCM() case?

> +
> + /* Write encryption iv */
> + qce_cpu_to_be32p_array(enciv, rctx->iv, enc_ivsize);
> + enciv_words = enc_ivsize / sizeof(u32);
> + qce_write_array(qce, REG_CNTR0_IV0, (u32 *)enciv, enciv_words);

It would be nice if this snippet was extracted to a helper function.

> +
> + if (IS_CCM(rctx->flags)) {
> + iv_last_word = (u32 *)[enciv_words - 1];
> +//   qce_write(qce, REG_CNTR3_IV3, enciv[enciv_words - 1] + 1);

I believe this is a remnant of the two surrounding lines.

> + qce_write(qce, REG_CNTR3_IV3, (*iv_last_word) + 1);

enciv is an array of big endian 32-bit integers, which you tell the
compiler to treat as cpu-native endian, and then you do math on it.
Afaict from the documentation the value of 

Re: [PATCH 1/7] crypto: qce: common: Add MAC failed error checking

2021-04-05 Thread Bjorn Andersson
On Thu 25 Feb 12:27 CST 2021, Thara Gopinath wrote:

> MAC_FAILED gets set in the status register if authenthication fails
> for ccm algorithms(during decryption). Add support to catch and flag
> this error.
> 
> Signed-off-by: Thara Gopinath 
> ---
>  drivers/crypto/qce/common.c | 11 ---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/crypto/qce/common.c b/drivers/crypto/qce/common.c
> index dceb9579d87a..7c3cb483749e 100644
> --- a/drivers/crypto/qce/common.c
> +++ b/drivers/crypto/qce/common.c
> @@ -403,7 +403,8 @@ int qce_start(struct crypto_async_request *async_req, u32 
> type)
>  }
>  
>  #define STATUS_ERRORS\
> - (BIT(SW_ERR_SHIFT) | BIT(AXI_ERR_SHIFT) | BIT(HSD_ERR_SHIFT))
> + (BIT(SW_ERR_SHIFT) | BIT(AXI_ERR_SHIFT) |   \
> +  BIT(HSD_ERR_SHIFT) | BIT(MAC_FAILED_SHIFT))
>  
>  int qce_check_status(struct qce_device *qce, u32 *status)
>  {
> @@ -417,8 +418,12 @@ int qce_check_status(struct qce_device *qce, u32 *status)
>* use result_status from result dump the result_status needs to be byte
>* swapped, since we set the device to little endian.
>*/
> - if (*status & STATUS_ERRORS || !(*status & BIT(OPERATION_DONE_SHIFT)))
> - ret = -ENXIO;
> + if (*status & STATUS_ERRORS || !(*status & BIT(OPERATION_DONE_SHIFT))) {
> + if (*status & BIT(MAC_FAILED_SHIFT))

Afaict MAC_FAILED indicates a different category of errors from the
others. So I would prefer that the conditionals are flattened.

Is OPERATION_DONE set when MAC_FAILED?

If so:

if (errors || !done)
return -ENXIO;
else if (*status & BIT(MAC_FAILED))
return -EBADMSG;

Would be cleaner in my opinion.

Regards,
Bjorn

> + ret = -EBADMSG;
> + else
> + ret = -ENXIO;
> + }
>  
>   return ret;
>  }
> -- 
> 2.25.1
> 


Re: [PATCH RESEND] arm64: dts: qcom: Add support for OnePlus 5/5T

2021-04-04 Thread Bjorn Andersson
On Mon 22 Mar 11:16 CDT 2021, Jami Kettunen wrote:
[..]
> diff --git a/arch/arm64/boot/dts/qcom/msm8998-oneplus-cheeseburger.dts 
> b/arch/arm64/boot/dts/qcom/msm8998-oneplus-cheeseburger.dts
> new file mode 100644
> index ..13b6b8ad4679
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8998-oneplus-cheeseburger.dts
> @@ -0,0 +1,42 @@
> +// SPDX-License-Identifier: GPL-2.0

The DT community prefer to get these under BSD-3-Clause instead? Would
you be willing to change this?

> +/*
> + * OnePlus 5 (cheeseburger) device tree
> + *
> + * Copyright (c) 2021, Jami Kettunen 
> + */
> +
[..]
> diff --git a/arch/arm64/boot/dts/qcom/msm8998-oneplus-common.dtsi 
> b/arch/arm64/boot/dts/qcom/msm8998-oneplus-common.dtsi
[..]
> +/* Disable all remoteprocs for now until RPM XO clock is usable */
> +_mss {

Can you please disable this in msm8998.dtsi and make devices explicitly
enable it instead - in a separate patch.

> + status = "disabled";
> +};
> +
[..]
> +/* Hold off on WLAN enablement until MSS remoteproc and friends are brought 
> up */

Are you saying leave disabled until MSS is up?

If so then move the comment inside the node and make it something like
"Leave disabled until MSS is functional" - to make it clearer that this
relates to the "status" of the node.

Regards,
Bjorn

> + {
> + vdd-0.8-cx-mx-supply = <_l5a_0p8>;
> + vdd-1.8-xo-supply = <_l7a_1p8>;
> + vdd-1.3-rfa-supply = <_l17a_1p3>;
> + vdd-3.3-ch0-supply = <_l25a_3p3>;
> +};
> diff --git a/arch/arm64/boot/dts/qcom/msm8998-oneplus-dumpling.dts 
> b/arch/arm64/boot/dts/qcom/msm8998-oneplus-dumpling.dts
> new file mode 100644
> index ..b46214a32478
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8998-oneplus-dumpling.dts
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * OnePlus 5T (dumpling) device tree
> + *
> + * Copyright (c) 2021, Jami Kettunen 
> + */
> +
> +#include "msm8998-oneplus-common.dtsi"
> +
> +/ {
> + model = "OnePlus 5T";
> + compatible = "oneplus,dumpling", "qcom,msm8998";
> + /* Required for bootloader to select correct board */
> + qcom,board-id = <8 0 17801 43>;
> +};
> +
> +/* Update the screen height values from 1920 to 2160 on the 5T */
> + {
> + height = <2160>;
> +};
> +
> +/* Adjust digitizer area height to match the 5T's taller panel */
> +_f12 {
> + touchscreen-y-mm = <137>;
> +};
> -- 
> 2.30.1
> 


Re: [PATCH V2 0/5] Add PMIC DT files for sc7280

2021-04-04 Thread Bjorn Andersson
On Thu 01 Apr 04:13 CDT 2021, satya priya wrote:

> Changes in V2:
>  - As per Matthias's comments:
> - I've Split the patch into per-PMIC patches and one sc7280 patch
> - Removed 2nd critical point, thermal-governer property
>   - s/pm8325_tz/pm7325_temp_alarm and s/pm7325_temp_alarm/pm7325_thermal
> - Fixed few other minor errors.
> 
>  - As per Bjorn's comments, replaced '_' with '-' in node names and moved
>DT files inclusion to board dts.
> 
> This series is dependent on below series which adds DT files for SC7280 SoC
> https://lore.kernel.org/patchwork/project/lkml/list/?series=488871

No need to mention this dependency, as you posted this after said series
had been picked up.

However, also picked up are patches from Vinod adding initial pm8350c
and pmk8350 files, so please rebase you changes onto linux-next - in
addition to follow up on Matthias feedback.

Thanks,
Bjorn

> 
> satya priya (5):
>   arm64: dts: qcom: pm7325: Add PMIC peripherals for pm7325
>   arm64: dts: qcom: pm8350c: Add PMIC peripherals for pm8350c
>   arm64: dts: qcom: pmk8350: Add PMIC peripherals for pmk8350
>   arm64: dts: qcom: pmr735a: Add PMIC peripherals for pmr735a
>   arm64: dts: sc7280: Include PMIC DT files for sc7280
> 
>  arch/arm64/boot/dts/qcom/pm7325.dtsi|  53 +
>  arch/arm64/boot/dts/qcom/pm8350c.dtsi   |  53 +
>  arch/arm64/boot/dts/qcom/pmk8350.dtsi   | 100 
> 
>  arch/arm64/boot/dts/qcom/pmr735a.dtsi   |  53 +
>  arch/arm64/boot/dts/qcom/sc7280-idp.dts |   4 ++
>  arch/arm64/boot/dts/qcom/sc7280.dtsi|   3 +
>  6 files changed, 266 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/qcom/pm7325.dtsi
>  create mode 100644 arch/arm64/boot/dts/qcom/pm8350c.dtsi
>  create mode 100644 arch/arm64/boot/dts/qcom/pmk8350.dtsi
>  create mode 100644 arch/arm64/boot/dts/qcom/pmr735a.dtsi
> 
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member 
> of Code Aurora Forum, hosted by The Linux Foundation
> 


Re: [PATCH V1 0/2] soc: qcom: aoss: Expose send for generic usecase

2021-04-04 Thread Bjorn Andersson
On Fri 02 Apr 01:17 CDT 2021, Deepak Kumar Singh wrote:

> [Change from V0]

It's typical that the first patchset, without a version specified, is
considered "version 1", and as such the second submission would be
"v2".

> Update qmp_get to parse qmp handle with binding qcom,qmp
> 

I won't be able to merge this until we have a user of the API, so would
it be possible to get at least one consumer introduced?

Regards,
Bjorn

> Deepak Kumar Singh (2):
>   soc: qcom: aoss: Expose send for generic usecase
>   soc: qcom: aoss: Add debugfs entry
> 
>  drivers/soc/qcom/qcom_aoss.c | 77 
> +++-
>  1 file changed, 76 insertions(+), 1 deletion(-)
> 
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 


Re: [PATCH V1 1/2] soc: qcom: aoss: Expose send for generic usecase

2021-04-04 Thread Bjorn Andersson
On Fri 02 Apr 01:17 CDT 2021, Deepak Kumar Singh wrote:

> Not all upcoming usecases will have an interface to allow the aoss
> driver to hook onto. Expose the send api and create a get function to
> enable drivers to send their own messages to aoss.
> 
> Signed-off-by: Chris Lew 
> Signed-off-by: Deepak Kumar Singh 
> ---
>  drivers/soc/qcom/qcom_aoss.c | 36 +++-
>  1 file changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/qcom/qcom_aoss.c b/drivers/soc/qcom/qcom_aoss.c
> index 53acb94..5c643f0 100644
> --- a/drivers/soc/qcom/qcom_aoss.c
> +++ b/drivers/soc/qcom/qcom_aoss.c
> @@ -8,10 +8,12 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 

I believe you forgot to 'git add' this.

>  
>  #define QMP_DESC_MAGIC   0x0
>  #define QMP_DESC_VERSION 0x4
> @@ -223,11 +225,14 @@ static bool qmp_message_empty(struct qmp *qmp)
>   *
>   * Return: 0 on success, negative errno on failure
>   */
> -static int qmp_send(struct qmp *qmp, const void *data, size_t len)
> +int qmp_send(struct qmp *qmp, const void *data, size_t len)
>  {
>   long time_left;
>   int ret;
>  
> + if (!qmp || !data)

I don't see a legit use case where these are NULL, so there's probably a
developer staring at the kernel log wondering why their code isn't
working. So better wrap this in a WARN_ON() to help him/her.


Also, a developer failing to check the return value of qmp_get() would
get here with qmp being -ENODEV, -EINVAL or -EPROBE_DEFER. Which we
would gladly dereference in the next conditional. So rather than !qmp,
IS_ERR_OR_NULL(qmp) would be useful.

> + return -EINVAL;
> +
>   if (WARN_ON(len + sizeof(u32) > qmp->size))
>   return -EINVAL;
>  
> @@ -261,6 +266,7 @@ static int qmp_send(struct qmp *qmp, const void *data, 
> size_t len)
>  
>   return ret;
>  }
> +EXPORT_SYMBOL(qmp_send);
>  
>  static int qmp_qdss_clk_prepare(struct clk_hw *hw)
>  {
> @@ -515,6 +521,34 @@ static void qmp_cooling_devices_remove(struct qmp *qmp)
>   thermal_cooling_device_unregister(qmp->cooling_devs[i].cdev);
>  }
>  
> +/**
> + * qmp_get() - get a qmp handle from a device
> + * @dev: client device pointer
> + *
> + * Return: handle to qmp device on success, ERR_PTR() on failure
> + */
> +struct qmp *qmp_get(struct device *dev)
> +{
> + struct platform_device *pdev;
> + struct device_node *np;
> + struct qmp *qmp;
> +
> + if (!dev || !dev->of_node)
> + return ERR_PTR(-ENODEV);

Value of @dev is an invalid argument, so I think -EINVAL is suitable.

> +
> + np = of_parse_phandle(dev->of_node, "qcom,qmp", 0);
> + if (!np)
> + return ERR_PTR(-ENODEV);
> +
> + pdev = of_find_device_by_node(np);

of_find_device_by_node() will increment the refcount of the underlying
struct device of pdev, so you need to platform_device_put() once you're
done with it.

As a side effect of not putting the struct device, the devm_kzalloc'ed
qmp pointer will remain valid. So care is needed to make sure that the
client doesn't end up with a dangling pointer if the qmp device is
removed.

My suggestion is that you add a "qmp_put()" function, which invokes
platform_device_put() and that you add some sort of tracking ("bool
orphan"?) to the struct qmp and make qmp_send() fail if this is set.

That way if someone unbinds the aoss device, the client will still have
a "valid" pointer, but won't be able to qmp_send() after qmp_close() has
been called in the aoss remove function.

Regards,
Bjorn

> + if (!pdev)
> + return ERR_PTR(-EINVAL);
> +
> + qmp = platform_get_drvdata(pdev);
> + return qmp ? qmp : ERR_PTR(-EPROBE_DEFER);
> +}
> +EXPORT_SYMBOL(qmp_get);
> +
>  static int qmp_probe(struct platform_device *pdev)
>  {
>   struct resource *res;
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 


Re: [PATCH V1 2/2] soc: qcom: aoss: Add debugfs entry

2021-04-04 Thread Bjorn Andersson
On Fri 02 Apr 01:17 CDT 2021, Deepak Kumar Singh wrote:

> It can be useful to control the different power states of various
> parts of hardware for device testing. Add a debugfs node for qmp so
> messages can be sent to aoss for debugging and testing purposes.
> 
> Signed-off-by: Chris Lew 
> Signed-off-by: Deepak Kumar Singh 
> ---
>  drivers/soc/qcom/qcom_aoss.c | 41 +
>  1 file changed, 41 insertions(+)
> 
> diff --git a/drivers/soc/qcom/qcom_aoss.c b/drivers/soc/qcom/qcom_aoss.c
> index 5c643f0..1789880 100644
> --- a/drivers/soc/qcom/qcom_aoss.c
> +++ b/drivers/soc/qcom/qcom_aoss.c
> @@ -4,6 +4,7 @@
>   */
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -86,6 +87,9 @@ struct qmp {
>   struct clk_hw qdss_clk;
>   struct genpd_onecell_data pd_data;
>   struct qmp_cooling_device *cooling_devs;
> +#if IS_ENABLED(CONFIG_DEBUG_FS)
> + struct dentry *debugfs_file;
> +#endif /* CONFIG_DEBUG_FS */
>  };
>  
>  struct qmp_pd {
> @@ -549,6 +553,34 @@ struct qmp *qmp_get(struct device *dev)
>  }
>  EXPORT_SYMBOL(qmp_get);
>  
> +#if IS_ENABLED(CONFIG_DEBUG_FS)
> +static ssize_t aoss_dbg_write(struct file *file, const char __user *userstr,
> +   size_t len, loff_t *pos)
> +{
> + struct qmp *qmp = file->private_data;
> + char buf[QMP_MSG_LEN] = {};
> + int ret;
> +
> + if (!len || len >= QMP_MSG_LEN)
> + return len;

If len >= QMP_MSG_LEN we shouldn't lie to the caller and say that all
went well, better return -EINVAL in this case.

> +
> + ret  = copy_from_user(buf, userstr, len);
> + if (ret) {
> + dev_err(qmp->dev, "copy from user failed, ret:%d\n", ret);
> + return len;

return -EFAULT;

And you don't have to print here.

The rest looks good.

Regards,
Bjorn

> + }
> +
> + ret = qmp_send(qmp, buf, QMP_MSG_LEN);
> +
> + return ret ? ret : len;
> +}
> +
> +static const struct file_operations aoss_dbg_fops = {
> + .open = simple_open,
> + .write = aoss_dbg_write,
> +};
> +#endif /* CONFIG_DEBUG_FS */
> +
>  static int qmp_probe(struct platform_device *pdev)
>  {
>   struct resource *res;
> @@ -603,6 +635,11 @@ static int qmp_probe(struct platform_device *pdev)
>  
>   platform_set_drvdata(pdev, qmp);
>  
> +#if IS_ENABLED(CONFIG_DEBUG_FS)
> + qmp->debugfs_file = debugfs_create_file("aoss_send_message", 0220, NULL,
> + qmp, _dbg_fops);
> +#endif /* CONFIG_DEBUG_FS */
> +
>   return 0;
>  
>  err_remove_qdss_clk:
> @@ -619,6 +656,10 @@ static int qmp_remove(struct platform_device *pdev)
>  {
>   struct qmp *qmp = platform_get_drvdata(pdev);
>  
> +#if IS_ENABLED(CONFIG_DEBUG_FS)
> + debugfs_remove(qmp->debugfs_file);
> +#endif /* CONFIG_DEBUG_FS */
> +
>   qmp_qdss_clk_remove(qmp);
>   qmp_pd_remove(qmp);
>   qmp_cooling_devices_remove(qmp);
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 


Re: [PATCH v2] thermal/drivers/tsens: fix missing put_device error

2021-04-04 Thread Bjorn Andersson
On Sun 04 Apr 07:54 CDT 2021, zhuguangqin...@gmail.com wrote:

> From: Guangqing Zhu 
> 
> Fixes coccicheck error:
> 
> drivers/thermal/qcom/tsens.c:759:4-10: ERROR: missing put_device; call
> of_find_device_by_node on line 715, but without a corresponding object
> release within this function.
> 
> Fixes: a7ff82976122 ("drivers: thermal: tsens: Merge tsens-common.c into
> tsens.c")
> 
> Signed-off-by: Guangqing Zhu 

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> ---
> v2:
>   - Fix a error(missing a bracket) in v1.
> 
> ---
>  drivers/thermal/qcom/tsens.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
> index d8ce3a687b80..3c4c0516e58a 100644
> --- a/drivers/thermal/qcom/tsens.c
> +++ b/drivers/thermal/qcom/tsens.c
> @@ -755,8 +755,10 @@ int __init init_common(struct tsens_priv *priv)
>   for (i = VER_MAJOR; i <= VER_STEP; i++) {
>   priv->rf[i] = devm_regmap_field_alloc(dev, 
> priv->srot_map,
> priv->fields[i]);
> - if (IS_ERR(priv->rf[i]))
> - return PTR_ERR(priv->rf[i]);
> + if (IS_ERR(priv->rf[i])) {
> + ret = PTR_ERR(priv->rf[i]);
> + goto err_put_device;
> + }
>   }
>   ret = regmap_field_read(priv->rf[VER_MINOR], _minor);
>   if (ret)
> -- 
> 2.17.1
> 


Re: [PATCH V2 3/3] scsi: ufs-qcom: configure VCC voltage level in vendor file

2021-04-01 Thread Bjorn Andersson
On Thu 01 Apr 09:58 CDT 2021, nitir...@codeaurora.org wrote:

> On 2021-03-31 23:49, Bjorn Andersson wrote:
> > On Wed 24 Mar 16:55 CDT 2021, nitir...@codeaurora.org wrote:
> > 
> > > On 2021-03-23 20:58, Bjorn Andersson wrote:
> > > > On Sun 21 Mar 16:57 CDT 2021, Nitin Rawat wrote:
> > > >
> > > > > As a part of vops handler, VCC voltage is updated
> > > > > as per the ufs device probed after reading the device
> > > > > descriptor. We follow below steps to configure voltage
> > > > > level.
> > > > >
> > > > > 1. Set the device to SLEEP state.
> > > > > 2. Disable the Vcc Regulator.
> > > > > 3. Set the vcc voltage according to the device type and reenable
> > > > >the regulator.
> > > > > 4. Set the device mode back to ACTIVE.
> > > > >
> > > >
> > > > When we discussed this a while back this was described as a requirement
> > > > from the device specification, you only operate on objects "owned" by
> > > > ufshcd and you invoke ufshcd operations to perform the actions.
> > > >
> > > > So why is this a ufs-qcom patch and not something in ufshcd?
> > > >
> > > > Regards,
> > > > Bjorn
> > > >
> > > > > Signed-off-by: Nitin Rawat 
> > > > > Signed-off-by: Veerabhadrarao Badiganti 
> > > > > ---
> > > > >  drivers/scsi/ufs/ufs-qcom.c | 51
> > > > > +
> > > > >  1 file changed, 51 insertions(+)
> > > > >
> > > > > diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
> > > > > index f97d7b0..ca35f5c 100644
> > > > > --- a/drivers/scsi/ufs/ufs-qcom.c
> > > > > +++ b/drivers/scsi/ufs/ufs-qcom.c
> > > > > @@ -21,6 +21,17 @@
> > > > >  #define UFS_QCOM_DEFAULT_DBG_PRINT_EN\
> > > > >   (UFS_QCOM_DBG_PRINT_REGS_EN | UFS_QCOM_DBG_PRINT_TEST_BUS_EN)
> > > > >
> > > > > +#define  ANDROID_BOOT_DEV_MAX30
> > > > > +static char android_boot_dev[ANDROID_BOOT_DEV_MAX];
> > > > > +
> > > > > +/* Min and Max VCC voltage values for ufs 2.x and
> > > > > + * ufs 3.x devices
> > > > > + */
> > > > > +#define UFS_3X_VREG_VCC_MIN_UV   254 /* uV */
> > > > > +#define UFS_3X_VREG_VCC_MAX_UV   270 /* uV */
> > > > > +#define UFS_2X_VREG_VCC_MIN_UV   295 /* uV */
> > > > > +#define UFS_2X_VREG_VCC_MAX_UV   296 /* uV */
> > > > > +
> > > > >  enum {
> > > > >   TSTBUS_UAWM,
> > > > >   TSTBUS_UARM,
> > > > > @@ -1293,6 +1304,45 @@ static void
> > > > > ufs_qcom_print_hw_debug_reg_all(struct ufs_hba *hba,
> > > > >   print_fn(hba, reg, 9, "UFS_DBG_RD_REG_TMRLUT ", priv);
> > > > >  }
> > > > >
> > > > > +  /**
> > > > > +   * ufs_qcom_setup_vcc_regulators - Update VCC voltage
> > > > > +   * @hba: host controller instance
> > > > > +   * Update VCC voltage based on UFS device(ufs 2.x or
> > > > > +   * ufs 3.x probed)
> > > > > +   */
> > > > > +static int ufs_qcom_setup_vcc_regulators(struct ufs_hba *hba)
> > > > > +{
> > > > > + struct ufs_dev_info *dev_info = >dev_info;
> > > > > + struct ufs_vreg *vreg = hba->vreg_info.vcc;
> > > > > + int ret;
> > > > > +
> > > > > + /* Put the device in sleep before lowering VCC level */
> > > > > + ret = ufshcd_set_dev_pwr_mode(hba, UFS_SLEEP_PWR_MODE);
> > > > > +
> > > > > + /* Switch off VCC before switching it ON at 2.5v or 2.96v */
> > > > > + ret = ufshcd_disable_vreg(hba->dev, vreg);
> > > > > +
> > > > > + /* add ~2ms delay before renabling VCC at lower voltage */
> > > > > + usleep_range(2000, 2100);
> > > > > +
> > > > > + /* set VCC min and max voltage according to ufs device type */
> > > > > + if (dev_info->wspecversion >= 0x300) {
> > > > > + vreg->min_uV = UFS_3X_VREG_VCC_MIN_UV;
> > > > > + vreg->max_uV = UFS_3X_VREG_VCC_MAX_UV;
>

Re: [PATCH V2 1/3] pinctrl: qcom: spmi-gpio: Add support for four variants

2021-04-01 Thread Bjorn Andersson
On Thu 01 Apr 07:35 CDT 2021, satya priya wrote:

> Add PM7325, PM8350c, PMK8350 and PMR735A compatibles for GPIO
> support.
> 
> Signed-off-by: satya priya 
> Acked-by: Bjorn Andersson 
> ---
> Changes in V2:
>  - No change.
> 
>  drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c 
> b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> index 9801c71..90f4f78 100644
> --- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> +++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> @@ -1131,6 +1131,10 @@ static const struct of_device_id pmic_gpio_of_match[] 
> = {
>   { .compatible = "qcom,pm6150l-gpio", .data = (void *) 12 },
>   /* pmx55 has 11 GPIOs with holes on 3, 7, 10, 11 */
>   { .compatible = "qcom,pmx55-gpio", .data = (void *) 11 },
> + { .compatible = "qcom,pm7325-gpio", .data = (void *) 10 },
> + { .compatible = "qcom,pm8350c-gpio", .data = (void *) 9 },
> + { .compatible = "qcom,pmk8350-gpio", .data = (void *) 4 },

Please try to keep the list sorted alphabetically, that way you'd see
that linux-next already has pm8350c and pmk8350 defined - or at least
Linus would notice when he tries to apply your patch.

Regards,
Bjorn

> + { .compatible = "qcom,pmr735a-gpio", .data = (void *) 4 },
>   { },
>  };
>  
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member 
> of Code Aurora Forum, hosted by The Linux Foundation
> 


Re: [PATCH V2 2/3] dt-bindings: pinctrl: qcom-pmic-gpio: Update the binding to add four new variants

2021-04-01 Thread Bjorn Andersson
On Thu 01 Apr 07:35 CDT 2021, satya priya wrote:

> Update the binding to add PM7325, PM8350C, PMK8350 and PMR735A GPIO support.
> 
> Signed-off-by: satya priya 
> ---
> Changes in V2:
>  - Placed this patch before conversion patch and updated commit text
>to be more clear.
> 
>  Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.txt | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.txt 
> b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.txt
> index 7648ab0..da7c35e 100644
> --- a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.txt
> @@ -30,6 +30,10 @@ PMIC's from Qualcomm.
>   "qcom,pm6150-gpio"
>   "qcom,pm6150l-gpio"
>   "qcom,pmx55-gpio"
> + "qcom,pm7325-gpio"
> + "qcom,pm8350c-gpio"
> + "qcom,pmk8350-gpio"
> + "qcom,pmr735a-gpio"

As with the driver, please try to keep these sorted alphabetically and
please rebase on linux-next, which already defines 2 of these.

Regards,
Bjorn

>  
>   And must contain either "qcom,spmi-gpio" or "qcom,ssbi-gpio"
>   if the device is on an spmi bus or an ssbi bus respectively
> @@ -113,6 +117,10 @@ to specify in a pin configuration subnode:
>   gpio1-gpio12 for pm6150l
>   gpio1-gpio11 for pmx55 (holes on gpio3, gpio7, gpio10
>   and gpio11)
> + gpio1-gpio10 for pm7325
> + gpio1-gpio9 for pm8350c
> + gpio1-gpio4 for pmk8350
> + gpio1-gpio4 for pmr735a
>  
>  - function:
>   Usage: required
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member 
> of Code Aurora Forum, hosted by The Linux Foundation
> 


Re: [PATCH v2] firmware: qcom_scm: Only compile legacy calls on ARM

2021-03-31 Thread Bjorn Andersson
On Tue 23 Mar 17:43 CDT 2021, Stephen Boyd wrote:

> These scm calls are never used outside of legacy ARMv7 based platforms.
> That's because PSCI, mandated on arm64, implements them for modern SoCs
> via the PSCI spec. Let's move them to the legacy file and only compile
> the legacy file into the kernel when CONFIG_ARM=y. Otherwise provide
> stubs and fail the calls. This saves a little bit of space in an
> arm64 allmodconfig.
> 
>  $ ./scripts/bloat-o-meter vmlinux.before vmlinux.after
>  add/remove: 0/8 grow/shrink: 5/6 up/down: 509/-4401 (-3892)
>  Function old new   delta
>  __qcom_scm_set_dload_mode.constprop  312 452+140
>  qcom_scm_qsmmu500_wait_safe_toggle   288 416+128
>  qcom_scm_io_writel   288 408+120
>  qcom_scm_io_readl376 492+116
>  __param_str_download_mode 23  28  +5
>  __warned43274326  -1
>  e843419@0b3f_00010432_324  8   -  -8
>  qcom_scm_call228 208 -20
>  CSWTCH  59255877 -48
>  _sub_I_65535_1163100  163040 -60
>  _sub_D_65535_0163100  163040 -60
>  qcom_scm_wb   64   - -64
>  qcom_scm_lock320 160-160
>  qcom_scm_call_atomic 212   --212
>  qcom_scm_cpu_power_down  308   --308
>  scm_legacy_call_atomic   520   --520
>  qcom_scm_set_warm_boot_addr  720   --720
>  qcom_scm_set_cold_boot_addr  728   --728
>  scm_legacy_call 1492   -   -1492
>  Total: Before=66737606, After=66733714, chg -0.01%
> 
> Commit 9a434cee773a ("firmware: qcom_scm: Dynamically support SMCCC and
> legacy conventions") didn't mention any motivating factors for keeping
> the legacy code around on arm64 kernels, i.e. presumably that commit
> wasn't trying to support these legacy APIs on arm64 kernels.
> 
> Cc: Elliot Berman 
> Cc: Brian Masney 
> Cc: Stephan Gerhold 
> Cc: Jeffrey Hugo 
> Cc: Douglas Anderson 
> Signed-off-by: Stephen Boyd 

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> ---
> 
> Followup to v1 
> (https://lore.kernel.org/r/20210223214539.1336155-7-swb...@chromium.org):
>  * Don't change the legacy file to use legacy calls only
>  * Wrap more things in CONFIG_ARM checks
> 
>  drivers/firmware/Makefile   |  4 +++-
>  drivers/firmware/qcom_scm.c | 47 -
>  drivers/firmware/qcom_scm.h | 15 
>  include/linux/qcom_scm.h| 21 ++---
>  4 files changed, 56 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
> index 5e013b6a3692..0b7b3a6c 100644
> --- a/drivers/firmware/Makefile
> +++ b/drivers/firmware/Makefile
> @@ -17,7 +17,9 @@ obj-$(CONFIG_ISCSI_IBFT)+= iscsi_ibft.o
>  obj-$(CONFIG_FIRMWARE_MEMMAP)+= memmap.o
>  obj-$(CONFIG_RASPBERRYPI_FIRMWARE) += raspberrypi.o
>  obj-$(CONFIG_FW_CFG_SYSFS)   += qemu_fw_cfg.o
> -obj-$(CONFIG_QCOM_SCM)   += qcom_scm.o qcom_scm-smc.o 
> qcom_scm-legacy.o
> +obj-$(CONFIG_QCOM_SCM)   += qcom_scm_objs.o
> +qcom_scm_objs-$(CONFIG_ARM)  += qcom_scm-legacy.o
> +qcom_scm_objs-$(CONFIG_QCOM_SCM) += qcom_scm.o qcom_scm-smc.o
>  obj-$(CONFIG_TI_SCI_PROTOCOL)+= ti_sci.o
>  obj-$(CONFIG_TRUSTED_FOUNDATIONS) += trusted_foundations.o
>  obj-$(CONFIG_TURRIS_MOX_RWTM)+= turris-mox-rwtm.o
> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> index ee9cb545e73b..747808a8ddf4 100644
> --- a/drivers/firmware/qcom_scm.c
> +++ b/drivers/firmware/qcom_scm.c
> @@ -49,28 +49,6 @@ struct qcom_scm_mem_map_info {
>   __le64 mem_size;
>  };
>  
> -#define QCOM_SCM_FLAG_COLDBOOT_CPU0  0x00
> -#define QCOM_SCM_FLAG_COLDBOOT_CPU1  0x01
> -#define QCOM_SCM_FLAG_COLDBOOT_CPU2  0x08
> -#define QCOM_SCM_FLAG_COLDBOOT_CPU3  0x20
> -
> -#define QCOM_SCM_FLAG_WARMBOOT_CPU0  0x04
> -#define QCOM_SCM_FLAG_WARMBOOT_CPU1  0x02
> -#define QCOM_SCM_FLAG_WARMBOOT_CPU2  0x10
> -#define QCOM_SCM_FLAG_WARMBOOT_CPU3  0x40
> -
> -struct qcom_scm_wb_entry {
> - int flag;
> - void *entry;
> -};
> -
> -static struct qcom_scm_wb_entry qcom_scm_wb[] = {
> - { .flag = QCOM_SCM_FLAG_WARMBOOT_CPU0 },
> - { .flag = QCOM_SCM_FLAG_WARMBOOT_CPU1 },
> - { .flag = QCOM_SCM_FLAG_WARMBOOT_CPU2 },

Re: [PATCH][next] soundwire: qcom: Fix a u8 comparison with less than zero

2021-03-31 Thread Bjorn Andersson
On Wed 31 Mar 09:09 CDT 2021, Colin King wrote:

> From: Colin Ian King 
> 
> Variable devnum is being checked for a less than zero error return
> however the comparison will always be false because devnum is an 8 bit
> unsigned integer. Fix this by making devnum an int.  Also there is no
> need to iniitialize devnum with zero as this value is no read, so
> remove the redundant assignment.
> 
> Addresses-Coverity: ("Unsigned compared against 0")
> Fixes: c7d49c76d1d5 ("soundwire: qcom: add support to new interrupts")
> Signed-off-by: Colin Ian King 

Reviewed-by: Bjorn Andersson 

> ---
>  drivers/soundwire/qcom.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index b08ecb9b418c..ec86c4e53fdb 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c
> @@ -428,7 +428,7 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void 
> *dev_id)
>   struct qcom_swrm_ctrl *swrm = dev_id;
>   u32 value, intr_sts, intr_sts_masked, slave_status;
>   u32 i;
> - u8 devnum = 0;
> + int devnum;
>   int ret = IRQ_HANDLED;
>  
>   swrm->reg_read(swrm, SWRM_INTERRUPT_STATUS, _sts);
> -- 
> 2.30.2
> 


Re: [PATCH V2 3/3] scsi: ufs-qcom: configure VCC voltage level in vendor file

2021-03-31 Thread Bjorn Andersson
On Wed 24 Mar 16:55 CDT 2021, nitir...@codeaurora.org wrote:

> On 2021-03-23 20:58, Bjorn Andersson wrote:
> > On Sun 21 Mar 16:57 CDT 2021, Nitin Rawat wrote:
> > 
> > > As a part of vops handler, VCC voltage is updated
> > > as per the ufs device probed after reading the device
> > > descriptor. We follow below steps to configure voltage
> > > level.
> > > 
> > > 1. Set the device to SLEEP state.
> > > 2. Disable the Vcc Regulator.
> > > 3. Set the vcc voltage according to the device type and reenable
> > >the regulator.
> > > 4. Set the device mode back to ACTIVE.
> > > 
> > 
> > When we discussed this a while back this was described as a requirement
> > from the device specification, you only operate on objects "owned" by
> > ufshcd and you invoke ufshcd operations to perform the actions.
> > 
> > So why is this a ufs-qcom patch and not something in ufshcd?
> > 
> > Regards,
> > Bjorn
> > 
> > > Signed-off-by: Nitin Rawat 
> > > Signed-off-by: Veerabhadrarao Badiganti 
> > > ---
> > >  drivers/scsi/ufs/ufs-qcom.c | 51
> > > +
> > >  1 file changed, 51 insertions(+)
> > > 
> > > diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
> > > index f97d7b0..ca35f5c 100644
> > > --- a/drivers/scsi/ufs/ufs-qcom.c
> > > +++ b/drivers/scsi/ufs/ufs-qcom.c
> > > @@ -21,6 +21,17 @@
> > >  #define UFS_QCOM_DEFAULT_DBG_PRINT_EN\
> > >   (UFS_QCOM_DBG_PRINT_REGS_EN | UFS_QCOM_DBG_PRINT_TEST_BUS_EN)
> > > 
> > > +#define  ANDROID_BOOT_DEV_MAX30
> > > +static char android_boot_dev[ANDROID_BOOT_DEV_MAX];
> > > +
> > > +/* Min and Max VCC voltage values for ufs 2.x and
> > > + * ufs 3.x devices
> > > + */
> > > +#define UFS_3X_VREG_VCC_MIN_UV   254 /* uV */
> > > +#define UFS_3X_VREG_VCC_MAX_UV   270 /* uV */
> > > +#define UFS_2X_VREG_VCC_MIN_UV   295 /* uV */
> > > +#define UFS_2X_VREG_VCC_MAX_UV   296 /* uV */
> > > +
> > >  enum {
> > >   TSTBUS_UAWM,
> > >   TSTBUS_UARM,
> > > @@ -1293,6 +1304,45 @@ static void
> > > ufs_qcom_print_hw_debug_reg_all(struct ufs_hba *hba,
> > >   print_fn(hba, reg, 9, "UFS_DBG_RD_REG_TMRLUT ", priv);
> > >  }
> > > 
> > > +  /**
> > > +   * ufs_qcom_setup_vcc_regulators - Update VCC voltage
> > > +   * @hba: host controller instance
> > > +   * Update VCC voltage based on UFS device(ufs 2.x or
> > > +   * ufs 3.x probed)
> > > +   */
> > > +static int ufs_qcom_setup_vcc_regulators(struct ufs_hba *hba)
> > > +{
> > > + struct ufs_dev_info *dev_info = >dev_info;
> > > + struct ufs_vreg *vreg = hba->vreg_info.vcc;
> > > + int ret;
> > > +
> > > + /* Put the device in sleep before lowering VCC level */
> > > + ret = ufshcd_set_dev_pwr_mode(hba, UFS_SLEEP_PWR_MODE);
> > > +
> > > + /* Switch off VCC before switching it ON at 2.5v or 2.96v */
> > > + ret = ufshcd_disable_vreg(hba->dev, vreg);
> > > +
> > > + /* add ~2ms delay before renabling VCC at lower voltage */
> > > + usleep_range(2000, 2100);
> > > +
> > > + /* set VCC min and max voltage according to ufs device type */
> > > + if (dev_info->wspecversion >= 0x300) {
> > > + vreg->min_uV = UFS_3X_VREG_VCC_MIN_UV;
> > > + vreg->max_uV = UFS_3X_VREG_VCC_MAX_UV;
> > > + }
> > > +
> > > + else {
> > > + vreg->min_uV = UFS_2X_VREG_VCC_MIN_UV;
> > > + vreg->max_uV = UFS_2X_VREG_VCC_MAX_UV;
> > > + }
> > > +
> > > + ret = ufshcd_enable_vreg(hba->dev, vreg);
> > > +
> > > + /* Bring the device in active now */
> > > + ret = ufshcd_set_dev_pwr_mode(hba, UFS_ACTIVE_PWR_MODE);
> > > + return ret;
> > > +}
> > > +
> > >  static void ufs_qcom_enable_test_bus(struct ufs_qcom_host *host)
> > >  {
> > >   if (host->dbg_print_en & UFS_QCOM_DBG_PRINT_TEST_BUS_EN) {
> > > @@ -1490,6 +1540,7 @@ static const struct ufs_hba_variant_ops
> > > ufs_hba_qcom_vops = {
> > >   .device_reset   = ufs_qcom_device_reset,
> > >   .config_scaling_param = ufs_qcom_config_scaling_param,
> > >   .program_key= ufs_qcom_ice_program_key,
> > > + .setup_vcc_regulators   = ufs_qcom_setup_vcc_regulators,
> > >  };
> > > 
> > >  /**
> > > --
> > > 2.7.4
> > > 
> 
> Hi Bjorn,
> Thanks for your review.
> But As per the earlier discussion regarding handling of vcc voltage
> for platform supporting both ufs 2.x and ufs 3.x , it was finally concluded
> to
> use "vops and let vendors handle it, until specs or someone
> has a better suggestion". Please correct me in case i am wrong.
> 

I was under the impression that this would result in something custom
per platform, but what I'm objecting to now that I see the code is that
this is completely generic.

And the concerns we discussed regarding these regulators being shared
with other devices is not considered in this implementation. But in
practice I don't see how you could support 2.x, 3.x and rail sharing at
the same time.

Regards,
Bjorn

> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg2399116.html
> 
> Regards,
> Nitin


Re: [PATCH V2] soundwire: qcom: use signed variable for error return

2021-03-31 Thread Bjorn Andersson
On Wed 31 Mar 10:55 CDT 2021, Vinod Koul wrote:

> We get warning of using a unsigned variable being compared to less than
> zero. The comparison is correct as it checks for errors from previous
> call to qcom_swrm_get_alert_slave_dev_num(), so we should use a signed
> variable here.
> 
> While at it, drop the superfluous initialization as well
> 
> drivers/soundwire/qcom.c: qcom_swrm_irq_handler() warn: impossible
> condition '(devnum < 0) => (0-255 < 0)'
> 
> Reported-by: kernel test robot 
> Signed-off-by: Vinod Koul 

Reviewed-by: Bjorn Andersson 

> ---
>  drivers/soundwire/qcom.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index b08ecb9b418c..ec86c4e53fdb 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c
> @@ -428,7 +428,7 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void 
> *dev_id)
>   struct qcom_swrm_ctrl *swrm = dev_id;
>   u32 value, intr_sts, intr_sts_masked, slave_status;
>   u32 i;
> - u8 devnum = 0;
> + int devnum;
>   int ret = IRQ_HANDLED;
>  
>   swrm->reg_read(swrm, SWRM_INTERRUPT_STATUS, _sts);
> -- 
> 2.26.3
> 


Re: [PATCH] soundwire: qcom: use signed variable for error return

2021-03-31 Thread Bjorn Andersson
On Wed 31 Mar 02:21 CDT 2021, Vinod Koul wrote:

> We get warning for using a unsigned variable being compared to less than
> zero. The comparison is correct as it checks for errors from previous
> call to qcom_swrm_get_alert_slave_dev_num(), so we should use a signed
> variable instead.
> 
> drivers/soundwire/qcom.c: qcom_swrm_irq_handler() warn: impossible
> condition '(devnum < 0) => (0-255 < 0)'
> 
> Reported-by: kernel test robot 
> Signed-off-by: Vinod Koul 
> ---
>  drivers/soundwire/qcom.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index b08ecb9b418c..55ed133c6704 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c
> @@ -428,7 +428,7 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void 
> *dev_id)
>   struct qcom_swrm_ctrl *swrm = dev_id;
>   u32 value, intr_sts, intr_sts_masked, slave_status;
>   u32 i;
> - u8 devnum = 0;
> + s8 devnum = 0;

At least in today's linux-next qcom_swrm_get_alert_slave_dev_num()
returns an int and the code only checks to see if this is negative.  So
it seems like making this a full int ensures there's no truncation etc.

And at least as written today there's no need to initialize the
variable.

Regards,
Bjorn

>   int ret = IRQ_HANDLED;
>  
>   swrm->reg_read(swrm, SWRM_INTERRUPT_STATUS, _sts);
> -- 
> 2.26.3
> 


Re: [PATCH v2] media: venus : hfi: add venus image info into smem

2021-03-29 Thread Bjorn Andersson
On Fri 26 Mar 01:33 CDT 2021, Dikshita Agarwal wrote:

> Fill fw version info into smem to be printed as part of
> soc info.
> 
> Signed-off-by: Dikshita Agarwal 
> 
> Changes since v1:
>  adressed comments from stephen.
>  removed unwanted code.
> ---
>  drivers/media/platform/qcom/venus/hfi_msgs.c | 21 +++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/hfi_msgs.c 
> b/drivers/media/platform/qcom/venus/hfi_msgs.c
> index 06a1908..6b6d33c9 100644
> --- a/drivers/media/platform/qcom/venus/hfi_msgs.c
> +++ b/drivers/media/platform/qcom/venus/hfi_msgs.c
> @@ -6,6 +6,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #include "core.h"
> @@ -14,6 +15,10 @@
>  #include "hfi_msgs.h"
>  #include "hfi_parser.h"
>  
> +#define SMEM_IMG_VER_TBL 469
> +#define VER_STR_SZ   128
> +#define SMEM_IMG_INDEX_VENUS 14 * 128

14 is the index, 128 is the element size, so this is now an "offset".

> +
>  static void event_seq_changed(struct venus_core *core, struct venus_inst 
> *inst,
> struct hfi_msg_event_notify_pkt *pkt)
>  {
> @@ -239,15 +244,27 @@ static void
>  sys_get_prop_image_version(struct device *dev,
>  struct hfi_msg_sys_property_info_pkt *pkt)
>  {
> + size_t smem_blk_sz = 0;

You shouldn't need to initialize smem_blk_sz if you check the return
value of qcom_smem_get() first.

> + u8 *smem_tbl_ptr;
> + u8 *img_ver;
>   int req_bytes;
>  
>   req_bytes = pkt->hdr.size - sizeof(*pkt);
>  
> - if (req_bytes < 128 || !pkt->data[1] || pkt->num_properties > 1)
> + if (req_bytes < VER_STR_SZ || !pkt->data[1] || pkt->num_properties > 1)
>   /* bad packet */
>   return;
>  
> - dev_dbg(dev, VDBGL "F/W version: %s\n", (u8 *)>data[1]);
> + img_ver = (u8 *)>data[1];
> +
> + dev_dbg(dev, VDBGL "F/W version: %s\n", img_ver);
> +
> + smem_tbl_ptr = qcom_smem_get(QCOM_SMEM_HOST_ANY,
> +SMEM_IMG_VER_TBL, _blk_sz);

80 chars is just a guideline and this looks prettier if you avoid the
line wrap. That said, if you pick shorter names for smem_tbl_ptr and
smem_blk_sz you probably even have to worry.

> + if ((SMEM_IMG_INDEX_VENUS + VER_STR_SZ) <= smem_blk_sz &&
> + smem_tbl_ptr)

In English you're trying to determine: "did qcom_smem_get() return a
valid pointer and is the item's size at least as big as we need".

So just write that in C:

if (smem_tbl_ptr && smem_blk_sz >= SMEM_IMG_INDEX_VENUS + VER_STR_SZ)

> + memcpy(smem_tbl_ptr + SMEM_IMG_INDEX_VENUS,
> +img_ver, VER_STR_SZ);

Again, please avoid the line wrap...

Regards,
Bjorn

>  }
>  
>  static void hfi_sys_property_info(struct venus_core *core,
> -- 
> 2.7.4
> 


Re: [PATCH v3 1/2] dt-bindings: thermal: qcom-tsens: Add compatible for sm8350

2021-03-29 Thread Bjorn Andersson
On Wed 24 Mar 07:43 CDT 2021, Robert Foss wrote:

> Add tsens bindings for sm8350.
> 
> Signed-off-by: Robert Foss 
> Reviewed-by: Vinod Koul 

Reviewed-by: Bjorn Andersson 

@Daniel, I presume it's better that you take this patch (1/2) through
your tree. I've picked patch 2.

Regards,
Bjorn

> ---
> 
> Changes since v1:
>  - Vinod: Remove comment
> 
>  Documentation/devicetree/bindings/thermal/qcom-tsens.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml 
> b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> index 95462e071ab4..e788378eff8d 100644
> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> @@ -43,6 +43,7 @@ properties:
>- qcom,sdm845-tsens
>- qcom,sm8150-tsens
>- qcom,sm8250-tsens
> +  - qcom,sm8350-tsens
>- const: qcom,tsens-v2
>  
>reg:
> -- 
> 2.31.0.30.g398dba342d.dirty
> 


Re: [PATCH 3/5] arm: dts: qcom: Add support for MSM8226 SoC

2021-03-29 Thread Bjorn Andersson
On Fri 26 Mar 09:58 CDT 2021, Bartosz Dudziak wrote:

> This patch adds basic device tree support for MSM8226 SoC which belongs
> to the Snapdragon 400 family. For now, this file adds the basic nodes
> like gcc, pinctrl and other required configuration for booting up to
> the serial console.
> 
> Signed-off-by: Bartosz Dudziak 
> ---
>  arch/arm/boot/dts/qcom-msm8226.dtsi | 152 
>  1 file changed, 152 insertions(+)
>  create mode 100644 arch/arm/boot/dts/qcom-msm8226.dtsi
> 
> diff --git a/arch/arm/boot/dts/qcom-msm8226.dtsi 
> b/arch/arm/boot/dts/qcom-msm8226.dtsi
> new file mode 100644
> index 00..81bb19398e
> --- /dev/null
> +++ b/arch/arm/boot/dts/qcom-msm8226.dtsi
> @@ -0,0 +1,152 @@
> +// SPDX-License-Identifier: GPL-2.0

Can you please make this BSD license?

> +/*
> + * Copyright (c) 2020, The Linux Foundation. All rights reserved.
> + */
> +
> +/dts-v1/;
> +
> +#include 
> +#include 
> +
> +/ {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + model = "Qualcomm Technologies, Inc. MSM8226";
> + compatible = "qcom,msm8226";

model and compatible should always be specified in the .dts, so the ones
specified here would be overwritten. So drop them here.

> + interrupt-parent = <>;
> +
> + chosen { };
> +
> + memory {
> + device_type = "memory";
> + /* We expect the bootloader to fill in the size */
> + reg = <0x0 0x0>;
> + };
> +
> + soc: soc {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> + compatible = "simple-bus";
> +
> + intc: interrupt-controller@f900 {
> + compatible = "qcom,msm-qgic2";
> + interrupt-controller;
> + #interrupt-cells = <3>;
> + reg = <0xF900 0x1000>,
> +   <0xF9002000 0x1000>;
> + };
> +
> + gcc: clock-controller@fc40 {
> + compatible = "qcom,gcc-msm8226";
> + #clock-cells = <1>;
> + #reset-cells = <1>;
> + #power-domain-cells = <1>;
> + reg = <0xfc40 0x4000>;
> + };
> +
> + msmgpio: pinctrl@fd51 {

Rename the label "tlmm"

> + compatible = "qcom,msm8226-pinctrl";
> + reg = <0xfd51 0x4000>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + gpio-ranges = < 0 0 117>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + interrupts = ;
> + };
> +
> + blsp1_uart3: serial@f991f000 {
> + compatible = "qcom,msm-uartdm-v1.4", "qcom,msm-uartdm";
> + reg = <0xf991f000 0x1000>;
> + interrupts = ;
> + clocks = < GCC_BLSP1_UART3_APPS_CLK>, < 
> GCC_BLSP1_AHB_CLK>;
> + clock-names = "core", "iface";
> + status = "disabled";
> + };
> +
> + restart@fc4ab000 {
> + compatible = "qcom,pshold";
> + reg = <0xfc4ab000 0x4>;
> + };
> +
> + rng@f9bff000 {
> + compatible = "qcom,prng";
> + reg = <0xf9bff000 0x200>;
> + clocks = < GCC_PRNG_AHB_CLK>;
> + clock-names = "core";
> + };
> +
> + timer@f902 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> + compatible = "arm,armv7-timer-mem";

It's nice to have compatible & reg first in the nodes.

Regards,
Bjorn

> + reg = <0xf902 0x1000>;
> + clock-frequency = <1920>;
> +
> + frame@f9021000 {
> + frame-number = <0>;
> + interrupts = ,
> +  ;
> + reg = <0xf9021000 0x1000>,
> +   <0xf9022000 0x1000>;
> + };
> +
> + frame@f9023000 {
> + frame-number = <1>;
> + interrupts = ;
> + reg = <0xf9023000 0x1000>;
> + status = "disabled";
> + };
> +
> + frame@f9024000 {
> + frame-number = <2>;
> + interrupts = ;
> + reg = <0xf9024000 0x1000>;
> + status = "disabled";
> + };
> +
> + frame@f9025000 {
> + frame-number = <3>;
> +

Re: [PATCH v5 7/7] arm64: dts: qcom: use dp_phy to provide clocks to dispcc

2021-03-29 Thread Bjorn Andersson
On Sun 28 Mar 15:52 CDT 2021, Dmitry Baryshkov wrote:

> Plug dp_phy-provided clocks to display clock controller.
> 

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> Signed-off-by: Dmitry Baryshkov 
> ---
>  arch/arm64/boot/dts/qcom/sm8250.dtsi | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi 
> b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index 0f79e6885004..a2478bd3590a 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -2600,8 +2600,8 @@ dispcc: clock-controller@af0 {
><_phy 1>,
><_phy 0>,
><_phy 1>,
> -  <0>,
> -  <0>,
> +  <_phy 0>,
> +  <_phy 1>,
><0>,
><0>,
><0>,
> @@ -2614,8 +2614,8 @@ dispcc: clock-controller@af0 {
> "dsi0_phy_pll_out_dsiclk",
> "dsi1_phy_pll_out_byteclk",
> "dsi1_phy_pll_out_dsiclk",
> -   "dp_link_clk_divsel_ten",
> -   "dp_vco_divided_clk_src_mux",
> +   "dp_phy_pll_link_clk",
> +   "dp_phy_pll_vco_div_clk",
> "dptx1_phy_pll_link_clk",
> "dptx1_phy_pll_vco_div_clk",
> "dptx2_phy_pll_link_clk",
> -- 
> 2.30.2
> 


Re: [PATCH v5 6/7] arm64: dts: qcom: sm8250: switch usb1 qmp phy to USB3+DP mode

2021-03-29 Thread Bjorn Andersson
On Sun 28 Mar 15:52 CDT 2021, Dmitry Baryshkov wrote:

> USB1 QMP PHY is not just a USB3 PHY, but USB3+DP PHY. Change device tree
> nodes accordingly.
> 
> Signed-off-by: Dmitry Baryshkov 

Reviewed-by: Bjorn Andersson 

@Vinod, will you let me know when you've picked the driver changes so I
can pick the two DT patches?

Regards,
Bjorn

> ---
>  arch/arm64/boot/dts/qcom/sm8250.dtsi | 23 ++-
>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi 
> b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index 947e1accae3a..0f79e6885004 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -2097,12 +2097,11 @@ usb_2_hsphy: phy@88e4000 {
>   };
>  
>   usb_1_qmpphy: phy@88e9000 {
> - compatible = "qcom,sm8250-qmp-usb3-phy";
> + compatible = "qcom,sm8250-qmp-usb3-dp-phy";
>   reg = <0 0x088e9000 0 0x200>,
> -   <0 0x088e8000 0 0x20>;
> - reg-names = "reg-base", "dp_com";
> +   <0 0x088e8000 0 0x40>,
> +   <0 0x088ea000 0 0x200>;
>   status = "disabled";
> - #clock-cells = <1>;
>   #address-cells = <2>;
>   #size-cells = <2>;
>   ranges;
> @@ -2116,7 +2115,7 @@ usb_1_qmpphy: phy@88e9000 {
>< GCC_USB3_PHY_PRIM_BCR>;
>   reset-names = "phy", "common";
>  
> - usb_1_ssphy: lanes@88e9200 {
> + usb_1_ssphy: usb3-phy@88e9200 {
>   reg = <0 0x088e9200 0 0x200>,
> <0 0x088e9400 0 0x200>,
> <0 0x088e9c00 0 0x400>,
> @@ -2128,6 +2127,20 @@ usb_1_ssphy: lanes@88e9200 {
>   clock-names = "pipe0";
>   clock-output-names = "usb3_phy_pipe_clk_src";
>   };
> +
> + dp_phy: dp-phy@88ea200 {
> + reg = <0 0x088ea200 0 0x200>,
> +   <0 0x088ea400 0 0x200>,
> +   <0 0x088eac00 0 0x400>,
> +   <0 0x088ea600 0 0x200>,
> +   <0 0x088ea800 0 0x200>,
> +   <0 0x088eaa00 0 0x100>;
> + #phy-cells = <0>;
> + #clock-cells = <1>;
> + clocks = < GCC_USB3_PRIM_PHY_PIPE_CLK>;
> + clock-names = "pipe0";
> + clock-output-names = "usb3_phy_pipe_clk_src";
> + };
>   };
>  
>   usb_2_qmpphy: phy@88eb000 {
> -- 
> 2.30.2
> 


Re: [PATCH v5 5/7] phy: qcom-qmp: add support for sm8250-usb3-dp phy

2021-03-29 Thread Bjorn Andersson
On Sun 28 Mar 15:52 CDT 2021, Dmitry Baryshkov wrote:

> Add support for QMP V4 Combo USB3+DP PHY (for SM8250 platform).
> 
> Signed-off-by: Dmitry Baryshkov 

Acked-by: Bjorn Andersson 

Regards,
Bjorn

> ---
>  drivers/phy/qualcomm/phy-qcom-qmp.c | 388 ++--
>  drivers/phy/qualcomm/phy-qcom-qmp.h |  40 ++-
>  2 files changed, 406 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c 
> b/drivers/phy/qualcomm/phy-qcom-qmp.c
> index 097bc005ba43..a47da2fff7a1 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
> @@ -1840,6 +1840,86 @@ static const struct qmp_phy_init_tbl 
> sm8250_usb3_uniphy_pcs_tbl[] = {
>   QMP_PHY_INIT_CFG(QPHY_V4_PCS_REFGEN_REQ_CONFIG1, 0x21),
>  };
>  
> +static const struct qmp_phy_init_tbl qmp_v4_dp_serdes_tbl[] = {
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_SVS_MODE_CLK_SEL, 0x05),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_SYSCLK_EN_SEL, 0x3b),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_SYS_CLK_CTRL, 0x02),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_CLK_ENABLE1, 0x0c),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_SYSCLK_BUF_ENABLE, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_CLK_SELECT, 0x30),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_PLL_IVCO, 0x0f),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_PLL_CCTRL_MODE0, 0x36),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_PLL_RCTRL_MODE0, 0x16),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_CP_CTRL_MODE0, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_CMN_CONFIG, 0x02),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_INTEGLOOP_GAIN0_MODE0, 0x3f),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_INTEGLOOP_GAIN1_MODE0, 0x00),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_VCO_TUNE_MAP, 0x00),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_DIV_FRAC_START1_MODE0, 0x00),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_BG_TIMER, 0x0a),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_CORECLK_DIV_MODE0, 0x0a),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_VCO_TUNE_CTRL, 0x00),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_BIAS_EN_CLKBUFLR_EN, 0x17),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_CORE_CLK_EN, 0x1f),
> +};
> +
> +static const struct qmp_phy_init_tbl qmp_v4_dp_serdes_tbl_rbr[] = {
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_HSCLK_SEL, 0x05),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_DEC_START_MODE0, 0x69),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_DIV_FRAC_START2_MODE0, 0x80),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_DIV_FRAC_START3_MODE0, 0x07),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_LOCK_CMP1_MODE0, 0x6f),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_LOCK_CMP2_MODE0, 0x08),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_LOCK_CMP_EN, 0x04),
> +};
> +
> +static const struct qmp_phy_init_tbl qmp_v4_dp_serdes_tbl_hbr[] = {
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_HSCLK_SEL, 0x03),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_DEC_START_MODE0, 0x69),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_DIV_FRAC_START2_MODE0, 0x80),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_DIV_FRAC_START3_MODE0, 0x07),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_LOCK_CMP1_MODE0, 0x0f),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_LOCK_CMP2_MODE0, 0x0e),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_LOCK_CMP_EN, 0x08),
> +};
> +
> +static const struct qmp_phy_init_tbl qmp_v4_dp_serdes_tbl_hbr2[] = {
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_HSCLK_SEL, 0x01),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_DEC_START_MODE0, 0x8c),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_DIV_FRAC_START2_MODE0, 0x00),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_DIV_FRAC_START3_MODE0, 0x0a),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_LOCK_CMP1_MODE0, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_LOCK_CMP2_MODE0, 0x1c),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_LOCK_CMP_EN, 0x08),
> +};
> +
> +static const struct qmp_phy_init_tbl qmp_v4_dp_serdes_tbl_hbr3[] = {
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_HSCLK_SEL, 0x00),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_DEC_START_MODE0, 0x69),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_DIV_FRAC_START2_MODE0, 0x80),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_DIV_FRAC_START3_MODE0, 0x07),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_LOCK_CMP1_MODE0, 0x2f),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_LOCK_CMP2_MODE0, 0x2a),
> + QMP_PHY_INIT_CFG(QSERDES_V4_COM_LOCK_CMP_EN, 0x08),
> +};
> +
> +static const struct qmp_phy_init_tbl qmp_v4_dp_tx_tbl[] = {
> + QMP_PHY_INIT_CFG(QSERDES_V4_TX_VMODE_CTRL1, 0x40),
> + QMP_PHY_INIT_CFG(QSERDES_V4_TX_PRE_STALL_LDO_BOOST_EN, 0x30),
> + QMP_PHY_INIT_CFG(QSERDES_V4_TX_INTERFACE_SELECT, 0x3b),
> + QMP_PHY_INIT_CFG(QSERDES_V4_TX_CLKBUF_ENABLE, 0x0f),
> + QMP_PHY_INIT_CFG(QSERDES_V4_TX_RESET_TSYNC_EN, 0x03),
> + QMP_PHY_INIT_CFG(QSERDES_V4_TX_TRAN_DRVR_EMP_EN, 0x0f),
> + QMP

Re: [PATCH v5 4/7] phy: qcom-qmp: rename common registers

2021-03-29 Thread Bjorn Andersson
On Sun 28 Mar 15:52 CDT 2021, Dmitry Baryshkov wrote:

> A plenty of DP PHY registers are common between V3 and V4. To simplify
> V4 code, rename all common registers.
> 
> Signed-off-by: Dmitry Baryshkov 

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> ---
>  drivers/phy/qualcomm/phy-qcom-qmp.c | 50 ++---
>  drivers/phy/qualcomm/phy-qcom-qmp.h | 37 ++---
>  2 files changed, 44 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c 
> b/drivers/phy/qualcomm/phy-qcom-qmp.c
> index 4150096fd350..097bc005ba43 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
> @@ -2435,20 +2435,20 @@ static void qcom_qmp_v3_phy_dp_aux_init(struct 
> qmp_phy *qphy)
>  {
>   writel(DP_PHY_PD_CTL_PWRDN | DP_PHY_PD_CTL_AUX_PWRDN |
>  DP_PHY_PD_CTL_PLL_PWRDN | DP_PHY_PD_CTL_DP_CLAMP_EN,
> -qphy->pcs + QSERDES_V3_DP_PHY_PD_CTL);
> +qphy->pcs + QSERDES_DP_PHY_PD_CTL);
>  
>   /* Turn on BIAS current for PHY/PLL */
>   writel(QSERDES_V3_COM_BIAS_EN | QSERDES_V3_COM_BIAS_EN_MUX |
>  QSERDES_V3_COM_CLKBUF_L_EN | QSERDES_V3_COM_EN_SYSCLK_TX_SEL,
>  qphy->serdes + QSERDES_V3_COM_BIAS_EN_CLKBUFLR_EN);
>  
> - writel(DP_PHY_PD_CTL_PSR_PWRDN, qphy->pcs + QSERDES_V3_DP_PHY_PD_CTL);
> + writel(DP_PHY_PD_CTL_PSR_PWRDN, qphy->pcs + QSERDES_DP_PHY_PD_CTL);
>  
>   writel(DP_PHY_PD_CTL_PWRDN | DP_PHY_PD_CTL_AUX_PWRDN |
>  DP_PHY_PD_CTL_LANE_0_1_PWRDN |
>  DP_PHY_PD_CTL_LANE_2_3_PWRDN | DP_PHY_PD_CTL_PLL_PWRDN |
>  DP_PHY_PD_CTL_DP_CLAMP_EN,
> -qphy->pcs + QSERDES_V3_DP_PHY_PD_CTL);
> +qphy->pcs + QSERDES_DP_PHY_PD_CTL);
>  
>   writel(QSERDES_V3_COM_BIAS_EN |
>  QSERDES_V3_COM_BIAS_EN_MUX | QSERDES_V3_COM_CLKBUF_R_EN |
> @@ -2456,16 +2456,16 @@ static void qcom_qmp_v3_phy_dp_aux_init(struct 
> qmp_phy *qphy)
>  QSERDES_V3_COM_CLKBUF_RX_DRIVE_L,
>  qphy->serdes + QSERDES_V3_COM_BIAS_EN_CLKBUFLR_EN);
>  
> - writel(0x00, qphy->pcs + QSERDES_V3_DP_PHY_AUX_CFG0);
> - writel(0x13, qphy->pcs + QSERDES_V3_DP_PHY_AUX_CFG1);
> - writel(0x24, qphy->pcs + QSERDES_V3_DP_PHY_AUX_CFG2);
> - writel(0x00, qphy->pcs + QSERDES_V3_DP_PHY_AUX_CFG3);
> - writel(0x0a, qphy->pcs + QSERDES_V3_DP_PHY_AUX_CFG4);
> - writel(0x26, qphy->pcs + QSERDES_V3_DP_PHY_AUX_CFG5);
> - writel(0x0a, qphy->pcs + QSERDES_V3_DP_PHY_AUX_CFG6);
> - writel(0x03, qphy->pcs + QSERDES_V3_DP_PHY_AUX_CFG7);
> - writel(0xbb, qphy->pcs + QSERDES_V3_DP_PHY_AUX_CFG8);
> - writel(0x03, qphy->pcs + QSERDES_V3_DP_PHY_AUX_CFG9);
> + writel(0x00, qphy->pcs + QSERDES_DP_PHY_AUX_CFG0);
> + writel(0x13, qphy->pcs + QSERDES_DP_PHY_AUX_CFG1);
> + writel(0x24, qphy->pcs + QSERDES_DP_PHY_AUX_CFG2);
> + writel(0x00, qphy->pcs + QSERDES_DP_PHY_AUX_CFG3);
> + writel(0x0a, qphy->pcs + QSERDES_DP_PHY_AUX_CFG4);
> + writel(0x26, qphy->pcs + QSERDES_DP_PHY_AUX_CFG5);
> + writel(0x0a, qphy->pcs + QSERDES_DP_PHY_AUX_CFG6);
> + writel(0x03, qphy->pcs + QSERDES_DP_PHY_AUX_CFG7);
> + writel(0xbb, qphy->pcs + QSERDES_DP_PHY_AUX_CFG8);
> + writel(0x03, qphy->pcs + QSERDES_DP_PHY_AUX_CFG9);
>   qphy->dp_aux_cfg = 0;
>  
>   writel(PHY_AUX_STOP_ERR_MASK | PHY_AUX_DEC_ERR_MASK |
> @@ -2556,9 +2556,9 @@ static int qcom_qmp_v3_phy_configure_dp_phy(struct 
> qmp_phy *qphy)
>*  writel(0x4c, qphy->pcs + QSERDES_V3_DP_PHY_MODE);
>*/
>   val |= DP_PHY_PD_CTL_LANE_2_3_PWRDN;
> - writel(val, qphy->pcs + QSERDES_V3_DP_PHY_PD_CTL);
> + writel(val, qphy->pcs + QSERDES_DP_PHY_PD_CTL);
>  
> - writel(0x5c, qphy->pcs + QSERDES_V3_DP_PHY_MODE);
> + writel(0x5c, qphy->pcs + QSERDES_DP_PHY_MODE);
>   writel(0x05, qphy->pcs + QSERDES_V3_DP_PHY_TX0_TX1_LANE_CTL);
>   writel(0x05, qphy->pcs + QSERDES_V3_DP_PHY_TX2_TX3_LANE_CTL);
>  
> @@ -2588,11 +2588,11 @@ static int qcom_qmp_v3_phy_configure_dp_phy(struct 
> qmp_phy *qphy)
>   clk_set_rate(dp_clks->dp_link_hw.clk, dp_opts->link_rate * 10);
>   clk_set_rate(dp_clks->dp_pixel_hw.clk, pixel_freq);
>  
> - writel(0x04, qphy->pcs + QSERDES_V3_DP_PHY_AUX_CFG2);
> - writel(0x01, qphy->pcs + QSERDES_V3_DP_PHY_CFG);
> - writel(0x05, qphy->pcs + QSERDES_V3_DP_PHY_CFG);
> - writel(0x01, qphy->pcs + QSERDES_V3_DP_PHY_CFG);
> - writel(0x09, qphy->pcs + QSERDES_V3_DP_PHY_CFG);
> + writel(0x04, qphy->pcs + QSERDES_DP_PHY_AUX_CFG2);
> 

Re: [PATCH v5 3/7] phy: qcom-qmp: move DP functions to callbacks

2021-03-29 Thread Bjorn Andersson
On Sun 28 Mar 15:52 CDT 2021, Dmitry Baryshkov wrote:

> In preparation to adding support for V4 DP PHY move DP functions to
> callbacks at struct qmp_phy_cfg.
> 

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> Signed-off-by: Dmitry Baryshkov 
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp.c | 438 +++-
>  1 file changed, 231 insertions(+), 207 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c 
> b/drivers/phy/qualcomm/phy-qcom-qmp.c
> index 9cdebe7f26cb..4150096fd350 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
> @@ -2268,6 +2268,8 @@ static const struct qmp_phy_init_tbl 
> sm8350_usb3_uniphy_pcs_tbl[] = {
>   QMP_PHY_INIT_CFG(QPHY_V4_PCS_REFGEN_REQ_CONFIG1, 0x21),
>  };
>  
> +struct qmp_phy;
> +
>  /* struct qmp_phy_cfg - per-PHY initialization config */
>  struct qmp_phy_cfg {
>   /* phy-type - PCIE/UFS/USB */
> @@ -2307,6 +2309,12 @@ struct qmp_phy_cfg {
>   const struct qmp_phy_init_tbl *serdes_tbl_hbr3;
>   int serdes_tbl_hbr3_num;
>  
> + /* DP PHY callbacks */
> + int (*configure_dp_phy)(struct qmp_phy *qphy);
> + void (*configure_dp_tx)(struct qmp_phy *qphy);
> + int (*calibrate_dp_phy)(struct qmp_phy *qphy);
> + void (*dp_aux_init)(struct qmp_phy *qphy);
> +
>   /* clock ids to be requested */
>   const char * const *clk_list;
>   int num_clks;
> @@ -2423,6 +2431,216 @@ struct qcom_qmp {
>   struct reset_control *ufs_reset;
>  };
>  
> +static void qcom_qmp_v3_phy_dp_aux_init(struct qmp_phy *qphy)
> +{
> + writel(DP_PHY_PD_CTL_PWRDN | DP_PHY_PD_CTL_AUX_PWRDN |
> +DP_PHY_PD_CTL_PLL_PWRDN | DP_PHY_PD_CTL_DP_CLAMP_EN,
> +qphy->pcs + QSERDES_V3_DP_PHY_PD_CTL);
> +
> + /* Turn on BIAS current for PHY/PLL */
> + writel(QSERDES_V3_COM_BIAS_EN | QSERDES_V3_COM_BIAS_EN_MUX |
> +QSERDES_V3_COM_CLKBUF_L_EN | QSERDES_V3_COM_EN_SYSCLK_TX_SEL,
> +qphy->serdes + QSERDES_V3_COM_BIAS_EN_CLKBUFLR_EN);
> +
> + writel(DP_PHY_PD_CTL_PSR_PWRDN, qphy->pcs + QSERDES_V3_DP_PHY_PD_CTL);
> +
> + writel(DP_PHY_PD_CTL_PWRDN | DP_PHY_PD_CTL_AUX_PWRDN |
> +DP_PHY_PD_CTL_LANE_0_1_PWRDN |
> +DP_PHY_PD_CTL_LANE_2_3_PWRDN | DP_PHY_PD_CTL_PLL_PWRDN |
> +DP_PHY_PD_CTL_DP_CLAMP_EN,
> +qphy->pcs + QSERDES_V3_DP_PHY_PD_CTL);
> +
> + writel(QSERDES_V3_COM_BIAS_EN |
> +QSERDES_V3_COM_BIAS_EN_MUX | QSERDES_V3_COM_CLKBUF_R_EN |
> +QSERDES_V3_COM_CLKBUF_L_EN | QSERDES_V3_COM_EN_SYSCLK_TX_SEL |
> +QSERDES_V3_COM_CLKBUF_RX_DRIVE_L,
> +qphy->serdes + QSERDES_V3_COM_BIAS_EN_CLKBUFLR_EN);
> +
> + writel(0x00, qphy->pcs + QSERDES_V3_DP_PHY_AUX_CFG0);
> + writel(0x13, qphy->pcs + QSERDES_V3_DP_PHY_AUX_CFG1);
> + writel(0x24, qphy->pcs + QSERDES_V3_DP_PHY_AUX_CFG2);
> + writel(0x00, qphy->pcs + QSERDES_V3_DP_PHY_AUX_CFG3);
> + writel(0x0a, qphy->pcs + QSERDES_V3_DP_PHY_AUX_CFG4);
> + writel(0x26, qphy->pcs + QSERDES_V3_DP_PHY_AUX_CFG5);
> + writel(0x0a, qphy->pcs + QSERDES_V3_DP_PHY_AUX_CFG6);
> + writel(0x03, qphy->pcs + QSERDES_V3_DP_PHY_AUX_CFG7);
> + writel(0xbb, qphy->pcs + QSERDES_V3_DP_PHY_AUX_CFG8);
> + writel(0x03, qphy->pcs + QSERDES_V3_DP_PHY_AUX_CFG9);
> + qphy->dp_aux_cfg = 0;
> +
> + writel(PHY_AUX_STOP_ERR_MASK | PHY_AUX_DEC_ERR_MASK |
> +PHY_AUX_SYNC_ERR_MASK | PHY_AUX_ALIGN_ERR_MASK |
> +PHY_AUX_REQ_ERR_MASK,
> +qphy->pcs + QSERDES_V3_DP_PHY_AUX_INTERRUPT_MASK);
> +}
> +
> +static const u8 qmp_dp_v3_pre_emphasis_hbr_rbr[4][4] = {
> + { 0x00, 0x0c, 0x14, 0x19 },
> + { 0x00, 0x0b, 0x12, 0xff },
> + { 0x00, 0x0b, 0xff, 0xff },
> + { 0x04, 0xff, 0xff, 0xff }
> +};
> +
> +static const u8 qmp_dp_v3_voltage_swing_hbr_rbr[4][4] = {
> + { 0x08, 0x0f, 0x16, 0x1f },
> + { 0x11, 0x1e, 0x1f, 0xff },
> + { 0x19, 0x1f, 0xff, 0xff },
> + { 0x1f, 0xff, 0xff, 0xff }
> +};
> +
> +static void qcom_qmp_v3_phy_configure_dp_tx(struct qmp_phy *qphy)
> +{
> + const struct phy_configure_opts_dp *dp_opts = >dp_opts;
> + unsigned int v_level = 0, p_level = 0;
> + u32 bias_en, drvr_en;
> + u8 voltage_swing_cfg, pre_emphasis_cfg;
> + int i;
> +
> + for (i = 0; i < dp_opts->lanes; i++) {
> + v_level = max(v_level, dp_opts->voltage[i]);
> + p_level = max(p_level, dp_opts->pre[i]);
> + }
> +
> + if (dp_opts->lanes == 1) {
> + bias_en = 0x3e;
> + drvr_en = 0x13;
> +

Re: [PATCH v5 2/7] dt-bindings: phy: qcom,qmp-usb3-dp: Add support for SM8250

2021-03-29 Thread Bjorn Andersson
On Sun 28 Mar 15:52 CDT 2021, Dmitry Baryshkov wrote:

> Add compatible for SM8250 in QMP USB3 DP PHY bindings.
> 
> Signed-off-by: Dmitry Baryshkov 
> Acked-by: Rob Herring 

Reviewed-by: Bjorn Andersson 

> ---
>  Documentation/devicetree/bindings/phy/qcom,qmp-usb3-dp-phy.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom,qmp-usb3-dp-phy.yaml 
> b/Documentation/devicetree/bindings/phy/qcom,qmp-usb3-dp-phy.yaml
> index 62c0179d1765..217aa6c91893 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,qmp-usb3-dp-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,qmp-usb3-dp-phy.yaml
> @@ -15,6 +15,7 @@ properties:
>  enum:
>- qcom,sc7180-qmp-usb3-dp-phy
>- qcom,sdm845-qmp-usb3-dp-phy
> +  - qcom,sm8250-qmp-usb3-dp-phy
>reg:
>  items:
>- description: Address and length of PHY's USB serdes block.
> -- 
> 2.30.2
> 


Re: [PATCH v5 1/7] dt-bindings: phy: qcom,qmp-usb3-dp-phy: move usb3 compatibles back to qcom,qmp-phy.yaml

2021-03-29 Thread Bjorn Andersson
On Sun 28 Mar 15:52 CDT 2021, Dmitry Baryshkov wrote:

> The commit 724fabf5df13 ("dt-bindings: phy: qcom,qmp-usb3-dp: Add DP phy
> information") has support for DP part of USB3+DP combo PHYs. However
> this change is not backwards compatible, placing additional requirements
> onto qcom,sc7180-qmp-usb3-phy and qcom,sdm845-qmp-usb3-phy device nodes
> (to include separate DP part, etc). However the aforementioned nodes do
> not inclue DP part, they strictly follow the schema defined in the
> qcom,qmp-phy.yaml file. Move those compatibles, leaving
> qcom,qmp-usb3-dp-phy.yaml to describe only real "combo" USB3+DP device nodes.
> 
> Fixes: 724fabf5df13 ("dt-bindings: phy: qcom,qmp-usb3-dp: Add DP phy 
> information")
> Cc: Stephen Boyd 
> Cc: Sandeep Maheswaram 
> Signed-off-by: Dmitry Baryshkov 
> Acked-by: Rob Herring 
> Reviewed-by: Stephen Boyd 

Reviewed-by: Bjorn Andersson 

> ---
>  Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml | 2 ++
>  Documentation/devicetree/bindings/phy/qcom,qmp-usb3-dp-phy.yaml | 2 --
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml 
> b/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml
> index 626447fee092..7808ec8bc712 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml
> @@ -25,11 +25,13 @@ properties:
>- qcom,msm8998-qmp-pcie-phy
>- qcom,msm8998-qmp-ufs-phy
>- qcom,msm8998-qmp-usb3-phy
> +  - qcom,sc7180-qmp-usb3-phy
>- qcom,sc8180x-qmp-ufs-phy
>- qcom,sc8180x-qmp-usb3-phy
>- qcom,sdm845-qhp-pcie-phy
>- qcom,sdm845-qmp-pcie-phy
>- qcom,sdm845-qmp-ufs-phy
> +  - qcom,sdm845-qmp-usb3-phy
>- qcom,sdm845-qmp-usb3-uni-phy
>- qcom,sm8150-qmp-ufs-phy
>- qcom,sm8150-qmp-usb3-phy
> diff --git a/Documentation/devicetree/bindings/phy/qcom,qmp-usb3-dp-phy.yaml 
> b/Documentation/devicetree/bindings/phy/qcom,qmp-usb3-dp-phy.yaml
> index 33974ad10afe..62c0179d1765 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,qmp-usb3-dp-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,qmp-usb3-dp-phy.yaml
> @@ -14,9 +14,7 @@ properties:
>compatible:
>  enum:
>- qcom,sc7180-qmp-usb3-dp-phy
> -  - qcom,sc7180-qmp-usb3-phy
>- qcom,sdm845-qmp-usb3-dp-phy
> -  - qcom,sdm845-qmp-usb3-phy
>reg:
>  items:
>- description: Address and length of PHY's USB serdes block.
> -- 
> 2.30.2
> 


Re: [PATCH 2/2] arm64: dts: msm8996: Mark the GPU's SMMU as an adreno one.

2021-03-29 Thread Bjorn Andersson
On Fri 26 Mar 18:13 CDT 2021, Eric Anholt wrote:

> This enables the adreno-specific SMMU path that sets HUPCF so
> (user-managed) page faults don't wedge the GPU.
> 
> Signed-off-by: Eric Anholt 

Acked-by: Bjorn Andersson 

@Will, can you pick this together with the driver patch? (So that they
land in order)

Regards,
Bjorn

> ---
> 
> We've been seeing a flaky test per day or so in Mesa CI where the
> kernel gets wedged after an iommu fault turns into CP errors.  With
> this patch, the CI isn't throwing the string of CP errors on the
> faults in any of the ~10 jobs I've run so far.
> 
>  arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi 
> b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> index 6de136e3add9..432b87ec9c5e 100644
> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> @@ -1127,7 +1127,7 @@ cci_i2c1: i2c-bus@1 {
>   };
>  
>   adreno_smmu: iommu@b4 {
> - compatible = "qcom,msm8996-smmu-v2", "qcom,smmu-v2";
> + compatible = "qcom,msm8996-smmu-v2", 
> "qcom,adreno-smmu", "qcom,smmu-v2";
>   reg = <0x00b4 0x1>;
>  
>   #global-interrupts = <1>;
> -- 
> 2.31.0
> 


Re: [PATCH 1/2] iommu/arm-smmu-qcom: Skip the TTBR1 quirk for db820c.

2021-03-29 Thread Bjorn Andersson
On Fri 26 Mar 18:13 CDT 2021, Eric Anholt wrote:

> db820c wants to use the qcom smmu path to get HUPCF set (which keeps
> the GPU from wedging and then sometimes wedging the kernel after a
> page fault), but it doesn't have separate pagetables support yet in
> drm/msm so we can't go all the way to the TTBR1 path.
> 
> Signed-off-by: Eric Anholt 

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> ---
> 
> We've been seeing a flaky test per day or so in Mesa CI where the
> kernel gets wedged after an iommu fault turns into CP errors.  With
> this patch, the CI isn't throwing the string of CP errors on the
> faults in any of the ~10 jobs I've run so far.
> 
>  drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c 
> b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index bcda17012aee..51f22193e456 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -130,6 +130,16 @@ static int qcom_adreno_smmu_alloc_context_bank(struct 
> arm_smmu_domain *smmu_doma
>   return __arm_smmu_alloc_bitmap(smmu->context_map, start, count);
>  }
>  
> +static bool qcom_adreno_can_do_ttbr1(struct arm_smmu_device *smmu)
> +{
> + const struct device_node *np = smmu->dev->of_node;
> +
> + if (of_device_is_compatible(np, "qcom,msm8996-smmu-v2"))
> + return false;
> +
> + return true;
> +}
> +
>  static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain,
>   struct io_pgtable_cfg *pgtbl_cfg, struct device *dev)
>  {
> @@ -144,7 +154,8 @@ static int qcom_adreno_smmu_init_context(struct 
> arm_smmu_domain *smmu_domain,
>* be AARCH64 stage 1 but double check because the arm-smmu code assumes
>* that is the case when the TTBR1 quirk is enabled
>*/
> - if ((smmu_domain->stage == ARM_SMMU_DOMAIN_S1) &&
> + if (qcom_adreno_can_do_ttbr1(smmu_domain->smmu) &&
> + (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) &&
>   (smmu_domain->cfg.fmt == ARM_SMMU_CTX_FMT_AARCH64))
>   pgtbl_cfg->quirks |= IO_PGTABLE_QUIRK_ARM_TTBR1;
>  
> -- 
> 2.31.0
> 


Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom

2021-03-26 Thread Bjorn Andersson
On Fri 26 Mar 10:24 CDT 2021, Rob Clark wrote:

> On Fri, Mar 26, 2021 at 8:18 AM Rob Clark  wrote:
> >
> > On Fri, Mar 26, 2021 at 5:38 AM Thierry Reding  
> > wrote:
> > >
> > > On Wed, Mar 17, 2021 at 06:53:04PM -0700, Rob Clark wrote:
> > > > On Wed, Mar 17, 2021 at 4:27 PM Matthias Kaehlcke  
> > > > wrote:
> > > > >
> > > > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote:
> > > > > > The sc7180-trogdor-pompom board might be attached to any number of a
> > > > > > pile of eDP panels. At the moment I'm told that the list might 
> > > > > > include:
> > > > > > - KD KD116N21-30NV-A010
> > > > > > - KD KD116N09-30NH-A016
> > > > > > - Starry 2081116HHD028001-51D
> > > > > > - Sharp LQ116M1JW10
> > > > > >
> > > > > > It should be noted that while the EDID programmed in the first 3
> > > > > > panels indicates that they should run with exactly the same timing 
> > > > > > (to
> > > > > > keep things simple), the 4th panel not only needs different timing 
> > > > > > but
> > > > > > has a different resolution.
> > > > > >
> > > > > > As is true in general with eDP panels, we can figure out which panel
> > > > > > we have and all the info needed to drive its pixel clock by reading
> > > > > > the EDID. However, we can do this only after we've powered the panel
> > > > > > on. Powering on the panels requires following the timing diagram in
> > > > > > each panel's datasheet which specifies delays between certain
> > > > > > actions. This means that, while we can be quite dynamic about 
> > > > > > handling
> > > > > > things we can't just totally skip out on describing the panel like 
> > > > > > we
> > > > > > could do if it was connected to an external-facing DP port.
> > > > > >
> > > > > > While the different panels have slightly different delays, it's
> > > > > > possible to come up with a set of unified delays that will work on 
> > > > > > all
> > > > > > the panels. From reading the datasheets:
> > > > > > * KD KD116N21-30NV-A010 and KD KD116N09-30NH-A016
> > > > > >   - HPD absent delay: 200 ms
> > > > > >   - Unprepare delay: 150 ms (datasheet is confusing, might be 500 
> > > > > > ms)
> > > > > > * Starry 2081116HHD028001-51D
> > > > > >   - HPD absent delay: 100 ms
> > > > > >   - Enable delay: (link training done till enable BL): 200 ms
> > > > > >   - Unprepare delay: 500 ms
> > > > > > * Sharp LQ116M1JW10
> > > > > >   - HPD absent delay: 200 ms
> > > > > >   - Unprepare delay: 500 ms
> > > > > >   - Prepare to enable delay (power on till backlight): 100 ms
> > > > > >
> > > > > > Unified:
> > > > > > - HPD absent delay: 200 ms
> > > > > > - Unprepare delay: 500 ms
> > > > > > - Enable delay: 200 ms
> > > > > >
> > > > > > NOTE: in theory the only thing that we _really_ need unity on is the
> > > > > > "HPD absent delay" since once the panel asserts HPD we can read the
> > > > > > EDID and could make per-panel decisions if we wanted.
> > > > > >
> > > > > > Let's create a definition of "a panel that can be attached to 
> > > > > > pompom"
> > > > > > as a panel that provides a valid EDID and can work with the standard
> > > > > > pompom power sequencing. If more panels are later attached to pompom
> > > > > > then it's fine as long as they work in a compatible way.
> > > > > >
> > > > > > One might ask why we can't just use a generic string here and 
> > > > > > provide
> > > > > > the timings directly in the device tree file. As I understand it,
> > > > > > trying to describe generic power sequencing in the device tree is
> > > > > > frowned upon and the one instance (SD/MMC) is regarded as a mistake
> > > > > > that shouldn't be repeated. Specifying a power sequence per board 
> > > > > > (or
> > > > > > per board class) feels like a reasonable compromise. We're not 
> > > > > > trying
> > > > > > to define fully generic power sequence bindings but we can also take
> > > > > > advantage of the semi-probable properties of the attached device.
> > > > > >
> > > > > > NOTE: I believe that past instances of supporting this type of thing
> > > > > > have used the "white lie" approach. One representative panel was
> > > > > > listed in the device tree. The power sequencings of this
> > > > > > representative panel were OK to use across all panels that might be
> > > > > > attached and other differences were handled by EDID. This patch
> > > > > > attempts to set a new precedent and avoid the need for the white 
> > > > > > lie.
> > > > > >
> > > > > > Signed-off-by: Douglas Anderson 
> > > > > > ---
> > > > >
> > > > > Sounds reasonable to me if DT maintainers can live with this abstract
> > > > > hardware definition. It's clearer than the 'white lie' approach.
> > > >
> > > > Yeah, it is a weird grey area between "discoverable" and "not
> > > > discoverable".. but I favor DT reflecting reality as much as
> > > > possible/feasible, so I think this is definity cleaner than "white
> > > > lies"
> > >
> > > This is practically no different than the "white lie". I suppose you
> > 

Re: [PATCH v2 1/2] arm64: dts: qcom: sdm845: Move reserved-memory to devices

2021-03-23 Thread Bjorn Andersson
On Fri 12 Mar 20:35 CST 2021, Konrad Dybcio wrote:

> Hi,
> 
> 
> I'm not sure I can agree. Especially for regions like IPA and
> TZ-reserved, which seem the same on (almost?) all..
> 

Thanks Konrad, I appreciate that.

> 
> Sure, the configuration for various remoteprocs *can* differ based on
> what the vendor decided to go with, but more often than not
> (especially with phones) vendors just take a MTP or CDP design, add a
> screen, couple of cameras and call it their own (you can tell by how
> similar most of them to the original reference designs in DT). While
> this is usually the case with lower-end (so not exactly sdm845)
> devices, it also kinda applies here...
> 

Unfortunately there's not a single memory map for each reference design,
the memory map do change during development based on feature set. I
think we can see this already among the few devices.

> 
> I guess for this one, we should find the lowest common denominator and
> keep the nodes that are in the majority of devices in 845 DTSI and
> only alter them if need be.. For WoA devices that may stray further
> away, you can just add a label to reserved-memory and /delete-node/
> it, so that you can rewrite it cleanly. The proposed approach just
> adds a lot - A LOT - of duplication. It will REALLY bite after more
> people submit 845-based phones, of which there are plenty (4 Xperias,
> a whole lot of Xiaomis, a couple of Samsungs, LGs... need I go on?).
> 

I was hoping to make it easier to reason about the memory map for each
device, but if your right about these incoming devices then I agree that
the duplication isn't worth it.

I'll respin patch 2, to get IPA going on the Yoga C630 and put this
patch on hold until this annoys me again :)

Thank you,
Bjorn


Re: [PATCH 6/6] firmware: qcom_scm: Only compile legacy calls on ARM

2021-03-23 Thread Bjorn Andersson
On Tue 23 Mar 13:27 CDT 2021, Elliot Berman wrote:

> On 3/22/2021 8:36 PM, Stephen Boyd wrote:
> > Quoting Bjorn Andersson (2021-03-07 09:42:45)
> > > On Sat 06 Mar 00:18 CST 2021, Stephen Boyd wrote:
> > > 
> > > > Quoting Elliot Berman (2021-03-05 10:18:09)
> > > > > On 3/3/2021 10:14 PM, Stephen Boyd wrote:
> > > > > > Quoting Elliot Berman (2021-03-03 19:35:08)
> > > > > 
> > > > >   > +desc.args[0] = flags;
> > > > >   > +desc.args[1] = virt_to_phys(entry);
> > > > >   > +
> > > > >   > +return scm_legacy_call_atomic(NULL, , NULL);
> > > > >   > +}
> > > > >   > +EXPORT_SYMBOL(qcom_scm_set_cold_boot_addr);
> > > > > 
> > > > > This should still be qcom_scm_call.
> > > > 
> > > > You mean s/scm_legacy_call_atomic/qcom_scm_call/ right?
> > > > 
> > > > I don't really want to resend the rest of the patches if this last one
> > > > is the only one that needs an update. This was a semi-RFC anyway so
> > > > maybe it's fine if the first 5 patches get merged and then I can resend
> > > > this one? Otherwise I will resend this again next week or so with less
> > > > diff for this patch.
> > > 
> > > I'm fine with merging the first 5, but was hoping that Elliot could
> > > provide either a "Reviewed-by" or at least an "Acked-by" on these.
> > > 
> > 
> > I'll take the silence as I should resend the whole series?
> > 
> 
> I thought Bjorn was accepting the first 5 already, my apologies.
> 
> Patches 1-5:
> Acked-by: Elliot Berman 

Thanks Elliot. I'll pick up the first 5, so please skip these in your
repost, Stephen.

Regards,
Bjorn


Re: [PATCH V2 3/3] scsi: ufs-qcom: configure VCC voltage level in vendor file

2021-03-23 Thread Bjorn Andersson
On Sun 21 Mar 16:57 CDT 2021, Nitin Rawat wrote:

> As a part of vops handler, VCC voltage is updated
> as per the ufs device probed after reading the device
> descriptor. We follow below steps to configure voltage
> level.
> 
> 1. Set the device to SLEEP state.
> 2. Disable the Vcc Regulator.
> 3. Set the vcc voltage according to the device type and reenable
>the regulator.
> 4. Set the device mode back to ACTIVE.
> 

When we discussed this a while back this was described as a requirement
from the device specification, you only operate on objects "owned" by
ufshcd and you invoke ufshcd operations to perform the actions.

So why is this a ufs-qcom patch and not something in ufshcd?

Regards,
Bjorn

> Signed-off-by: Nitin Rawat 
> Signed-off-by: Veerabhadrarao Badiganti 
> ---
>  drivers/scsi/ufs/ufs-qcom.c | 51 
> +
>  1 file changed, 51 insertions(+)
> 
> diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
> index f97d7b0..ca35f5c 100644
> --- a/drivers/scsi/ufs/ufs-qcom.c
> +++ b/drivers/scsi/ufs/ufs-qcom.c
> @@ -21,6 +21,17 @@
>  #define UFS_QCOM_DEFAULT_DBG_PRINT_EN\
>   (UFS_QCOM_DBG_PRINT_REGS_EN | UFS_QCOM_DBG_PRINT_TEST_BUS_EN)
> 
> +#define  ANDROID_BOOT_DEV_MAX30
> +static char android_boot_dev[ANDROID_BOOT_DEV_MAX];
> +
> +/* Min and Max VCC voltage values for ufs 2.x and
> + * ufs 3.x devices
> + */
> +#define UFS_3X_VREG_VCC_MIN_UV   254 /* uV */
> +#define UFS_3X_VREG_VCC_MAX_UV   270 /* uV */
> +#define UFS_2X_VREG_VCC_MIN_UV   295 /* uV */
> +#define UFS_2X_VREG_VCC_MAX_UV   296 /* uV */
> +
>  enum {
>   TSTBUS_UAWM,
>   TSTBUS_UARM,
> @@ -1293,6 +1304,45 @@ static void ufs_qcom_print_hw_debug_reg_all(struct 
> ufs_hba *hba,
>   print_fn(hba, reg, 9, "UFS_DBG_RD_REG_TMRLUT ", priv);
>  }
> 
> +  /**
> +   * ufs_qcom_setup_vcc_regulators - Update VCC voltage
> +   * @hba: host controller instance
> +   * Update VCC voltage based on UFS device(ufs 2.x or
> +   * ufs 3.x probed)
> +   */
> +static int ufs_qcom_setup_vcc_regulators(struct ufs_hba *hba)
> +{
> + struct ufs_dev_info *dev_info = >dev_info;
> + struct ufs_vreg *vreg = hba->vreg_info.vcc;
> + int ret;
> +
> + /* Put the device in sleep before lowering VCC level */
> + ret = ufshcd_set_dev_pwr_mode(hba, UFS_SLEEP_PWR_MODE);
> +
> + /* Switch off VCC before switching it ON at 2.5v or 2.96v */
> + ret = ufshcd_disable_vreg(hba->dev, vreg);
> +
> + /* add ~2ms delay before renabling VCC at lower voltage */
> + usleep_range(2000, 2100);
> +
> + /* set VCC min and max voltage according to ufs device type */
> + if (dev_info->wspecversion >= 0x300) {
> + vreg->min_uV = UFS_3X_VREG_VCC_MIN_UV;
> + vreg->max_uV = UFS_3X_VREG_VCC_MAX_UV;
> + }
> +
> + else {
> + vreg->min_uV = UFS_2X_VREG_VCC_MIN_UV;
> + vreg->max_uV = UFS_2X_VREG_VCC_MAX_UV;
> + }
> +
> + ret = ufshcd_enable_vreg(hba->dev, vreg);
> +
> + /* Bring the device in active now */
> + ret = ufshcd_set_dev_pwr_mode(hba, UFS_ACTIVE_PWR_MODE);
> + return ret;
> +}
> +
>  static void ufs_qcom_enable_test_bus(struct ufs_qcom_host *host)
>  {
>   if (host->dbg_print_en & UFS_QCOM_DBG_PRINT_TEST_BUS_EN) {
> @@ -1490,6 +1540,7 @@ static const struct ufs_hba_variant_ops 
> ufs_hba_qcom_vops = {
>   .device_reset   = ufs_qcom_device_reset,
>   .config_scaling_param = ufs_qcom_config_scaling_param,
>   .program_key= ufs_qcom_ice_program_key,
> + .setup_vcc_regulators   = ufs_qcom_setup_vcc_regulators,
>  };
> 
>  /**
> --
> 2.7.4
> 


Re: [PATCH] pinctrl: qcom: fix unintentional string concatenation

2021-03-23 Thread Bjorn Andersson
On Tue 23 Mar 08:17 CDT 2021, Arnd Bergmann wrote:

> From: Arnd Bergmann 
> 
> clang is clearly correct to point out a typo in a silly
> array of strings:
> 
> drivers/pinctrl/qcom/pinctrl-sdx55.c:426:61: error: suspicious concatenation 
> of string literals in an array initialization; did you mean to separate the 
> elements with a comma? [-Werror,-Wstring-concatenation]
> "gpio14", "gpio15", "gpio16", "gpio17", "gpio18", "gpio19" "gpio20", 
> "gpio21", "gpio22",
>^
> Add the missing comma that must have accidentally been removed.

That's certainly a useful warning! Thanks Arnd.

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> 
> Fixes: ac43c44a7a37 ("pinctrl: qcom: Add SDX55 pincontrol driver")
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/pinctrl/qcom/pinctrl-sdx55.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/qcom/pinctrl-sdx55.c 
> b/drivers/pinctrl/qcom/pinctrl-sdx55.c
> index 2b5b0e2b03ad..5aaf57b40407 100644
> --- a/drivers/pinctrl/qcom/pinctrl-sdx55.c
> +++ b/drivers/pinctrl/qcom/pinctrl-sdx55.c
> @@ -423,7 +423,7 @@ static const char * const gpio_groups[] = {
>  
>  static const char * const qdss_stm_groups[] = {
>   "gpio0", "gpio1", "gpio2", "gpio3", "gpio4", "gpio5", "gpio6", "gpio7", 
> "gpio12", "gpio13",
> - "gpio14", "gpio15", "gpio16", "gpio17", "gpio18", "gpio19" "gpio20", 
> "gpio21", "gpio22",
> + "gpio14", "gpio15", "gpio16", "gpio17", "gpio18", "gpio19", "gpio20", 
> "gpio21", "gpio22",
>   "gpio23", "gpio44", "gpio45", "gpio52", "gpio53", "gpio56", "gpio57", 
> "gpio61", "gpio62",
>   "gpio63", "gpio64", "gpio65", "gpio66",
>  };
> -- 
> 2.29.2
> 


Re: [PATCH 0/5] qcom: wcnss: Allow overriding firmware form DT

2021-03-18 Thread Bjorn Andersson
On Thu 18 Mar 11:56 CDT 2021, Jeffrey Hugo wrote:

> form -> from in the subject?
> 

Seems like I only failed in the cover letter, right?

Regards,
Bjorn

> On Thu, Mar 11, 2021 at 5:34 PM Bjorn Andersson
>  wrote:
> >
> > The wireless subsystem found in Qualcomm MSM8974 and MSM8916 among others 
> > needs
> > platform-, and perhaps even board-, specific firmware. Add support for
> > providing this in devicetree.


Re: [PATCH 5/5] arm64: dts: qcom: msm8916: Enable modem and WiFi

2021-03-18 Thread Bjorn Andersson
On Mon 15 Mar 07:01 CDT 2021, Bryan O'Donoghue wrote:

> On 12/03/2021 00:33, Bjorn Andersson wrote:
> > Enable the modem and WiFi subsystems and specify msm8916 specific
> > firmware path for these and the WCNSS control service.
> > 
> > Signed-off-by: Bjorn Andersson 
> > ---
> >   arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 12 
> >   arch/arm64/boot/dts/qcom/msm8916.dtsi |  2 +-
> >   2 files changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi 
> > b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
> > index 6aef0c2e4f0a..448e3561ef63 100644
> > --- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
> > @@ -305,6 +305,12 @@  {
> > status = "okay";
> >   };
> > + {
> > +   status = "okay";
> > +
> > +   firmware-name = "qcom/msm8916/mba.mbn", "qcom/msm8916/modem.mbn";
> > +};
> > +
> >   _resin {
> > status = "okay";
> > linux,code = ;
> > @@ -312,6 +318,8 @@ _resin {
> >{
> > status = "okay";
> > +
> > +   firmware-name = "qcom/msm8916/wcnss.mbn";
> >   };
> 
> On Debian I have to do this
> 
> 
> index 2a6a23cb14ca..597cdc8f51cc 100644
> --- a/drivers/remoteproc/qcom_wcnss.c
> +++ b/drivers/remoteproc/qcom_wcnss.c
> @@ -33,7 +33,7 @@
>  #include "qcom_wcnss.h"
> 
>  #define WCNSS_CRASH_REASON_SMEM422
> -#define WCNSS_FIRMWARE_NAME"wcnss.mdt"
> +#define WCNSS_FIRMWARE_NAME"qcom/msm8916/wcnss.mdt"
> 
> so I guess wcnss_probe() -> rproc_alloc() wants this fix too.
> 

Can you confirm that you're saying that you want below patch, which I
just merged?

https://lore.kernel.org/linux-remoteproc/20210312002441.3273183-1-bjorn.anders...@linaro.org/

(Which makes it possible to specify firmware name per platform/board)

Regards,
Bjorn


Re: [PATCH 3/5] dt-bindings: remoteproc: Add the documentation for Meson AO ARC rproc

2021-03-17 Thread Bjorn Andersson
On Tue 29 Dec 19:27 CST 2020, Martin Blumenstingl wrote:

> Amlogic Meson6, Meson8, Meson8b and Meson8m2 SoCs embed an ARC EM4
> controller for always-on operations, typically used for managing system
> suspend.
> 
> Signed-off-by: Martin Blumenstingl 
> ---
>  .../remoteproc/amlogic,meson-mx-ao-arc.yaml   | 87 +++
>  1 file changed, 87 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/remoteproc/amlogic,meson-mx-ao-arc.yaml
> 
> diff --git 
> a/Documentation/devicetree/bindings/remoteproc/amlogic,meson-mx-ao-arc.yaml 
> b/Documentation/devicetree/bindings/remoteproc/amlogic,meson-mx-ao-arc.yaml
> new file mode 100644
> index ..ba5deebaf7dc
> --- /dev/null
> +++ 
> b/Documentation/devicetree/bindings/remoteproc/amlogic,meson-mx-ao-arc.yaml
> @@ -0,0 +1,87 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/remoteproc/amlogic,meson-mx-ao-arc.yaml#;
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#;
> +
> +title: Amlogic Meson AO ARC Remote Processor bindings
> +
> +description:
> +  Amlogic Meson6, Meson8, Meson8b and Meson8m2 SoCs embed an ARC core
> +  controller for always-on operations, typically used for managing
> +  system suspend. Meson6 and older use a ARC core based on the ARCv1
> +  ISA, while Meson8, Meson8b and Meson8m2 use an ARC EM4 (ARCv2 ISA)
> +  core.
> +
> +maintainers:
> +  - Martin Blumenstingl 
> +
> +properties:
> +  compatible:
> +items:
> +  - enum:
> +- amlogic,meson8-ao-arc
> +- amlogic,meson8b-ao-arc
> +  - const: amlogic,meson-mx-ao-arc
> +
> +  firmware-name:
> +$ref: /schemas/types.yaml#/definitions/string
> +description:
> +  The name of the firmware which should be loaded for this remote
> +  processor.
> +
> +  reg:
> +description:
> +  Address ranges of the remap and CPU control addresses for the
> +  remote processor.
> +minItems: 2
> +
> +  reg-names:
> +items:
> +  - const: remap
> +  - const: cpu
> +
> +  resets:
> + minItems: 1
> +
> +  clocks:
> +minItems: 1
> +
> +  sram:
> +$ref: /schemas/types.yaml#/definitions/phandle
> +description:
> +  phandles to a reserved SRAM region which is used as the memory of
> +  the ARC core. The region should be defined as child nodes of the
> +  AHB SRAM node as per the generic bindings in
> +  Documentation/devicetree/bindings/sram/sram.yaml
> +
> +  amlogic,secbus2:
> +$ref: /schemas/types.yaml#/definitions/phandle
> +description:
> +  A phandle to the SECBUS2 region which contains some configuration
> +  bits of this remote processor
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - resets
> +  - clocks
> +  - sram
> +  - amlogic,secbus2
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +remoteproc@1c {
> +  compatible= "amlogic,meson8-ao-arc", "amlogic,meson-mx-ao-arc";
> +  reg = <0x1c 0x8>, <0x38 0x8>;

I'm generally not in favor of mapping "individual" registers, do you
know what hardware block this is part of? Can you express the whole
block as an single entity in your DT?

Regards,
Bjorn

> +  reg-names = "remap", "cpu";
> +  resets = <_cpu_reset>;
> +  clocks = <_cpu_clock>;
> +  sram = <_sram_ao_arc>;
> +  amlogic,secbus2 = <>;
> +};
> +
> +...
> -- 
> 2.30.0
> 


Re: [PATCH 4/5] remoteproc: meson-mx-ao-arc: Add a driver for the AO ARC remote procesor

2021-03-17 Thread Bjorn Andersson
On Tue 29 Dec 19:27 CST 2020, Martin Blumenstingl wrote:

> Amlogic Meson6, Meson8, Meson8b and Meson8m2 embed an ARC core in the
> Always-On (AO) power-domain. This is typically used for waking up the
> ARM cores after system suspend.
> 
> The configuration is spread across three different registers:
> - AO_REMAP_REG0 which must be programmed to zero, it's actual purpose
>   is unknown. There is a second remap register which is not used in the
>   vendor kernel (which served as reference for this driver).
> - AO_CPU_CNTL is used to start and stop the ARC core.
> - AO_SECURE_REG0 in the SECBUS2 register area with unknown purpose.
> 
> To boot the ARC core we also need to enable it's gate clock and trigger
> a reset.
> 
> The actual code for this ARC core can come from an ELF binary, for
> example by building the Zephyr RTOS for an ARC EM4 core and then taking
> "zephyr.elf" as firmware. This executable does not have any "rsc table"
> so we are skipping rproc_elf_load_rsc_table (rproc_ops.parse_fw) and
> rproc_elf_find_loaded_rsc_table (rproc_ops.find_loaded_rsc_table).
> 

Thanks for the patch Martin, it looks really good. Just some minor
things as I expect a respin of the DT binding as well.

> Signed-off-by: Martin Blumenstingl 
> ---
>  drivers/remoteproc/Kconfig   |  11 ++
>  drivers/remoteproc/Makefile  |   1 +
>  drivers/remoteproc/meson_mx_ao_arc.c | 240 +++
>  3 files changed, 252 insertions(+)
>  create mode 100644 drivers/remoteproc/meson_mx_ao_arc.c
> 
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 9e7efe542f69..0e7fb91635fe 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -125,6 +125,17 @@ config KEYSTONE_REMOTEPROC
> It's safe to say N here if you're not interested in the Keystone
> DSPs or just want to use a bare minimum kernel.
>  
> +config MESON_MX_AO_ARC_REMOTEPROC
> + tristate "Amlogic Meson6/8/8b/8m2 AO ARC remote processor support"
> + depends on HAS_IOMEM
> + depends on (ARM && ARCH_MESON) || COMPILE_TEST
> + select GENERIC_ALLOCATOR
> + help
> +   Say m or y here to have support for the AO ARC remote processor
> +   on Amlogic Meson6/Meson8/Meson8b/Meson8m2 SoCs. This is
> +   typically used for system suspend.
> +   If unusre say N.
> +
>  config PRU_REMOTEPROC
>   tristate "TI PRU remoteproc support"
>   depends on TI_PRUSS
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index bb26c9e4ef9c..ce1abeb30907 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_OMAP_REMOTEPROC)   += 
> omap_remoteproc.o
>  obj-$(CONFIG_WKUP_M3_RPROC)  += wkup_m3_rproc.o
>  obj-$(CONFIG_DA8XX_REMOTEPROC)   += da8xx_remoteproc.o
>  obj-$(CONFIG_KEYSTONE_REMOTEPROC)+= keystone_remoteproc.o
> +obj-$(CONFIG_MESON_MX_AO_ARC_REMOTEPROC)+= meson_mx_ao_arc.o
>  obj-$(CONFIG_PRU_REMOTEPROC) += pru_rproc.o
>  obj-$(CONFIG_QCOM_PIL_INFO)  += qcom_pil_info.o
>  obj-$(CONFIG_QCOM_RPROC_COMMON)  += qcom_common.o
> diff --git a/drivers/remoteproc/meson_mx_ao_arc.c 
> b/drivers/remoteproc/meson_mx_ao_arc.c
> new file mode 100644
> index ..1deb03ca30f4
> --- /dev/null
> +++ b/drivers/remoteproc/meson_mx_ao_arc.c
> @@ -0,0 +1,240 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2020 Martin Blumenstingl 
> 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "remoteproc_internal.h"
> +
> +#define AO_REMAP_REG00x0
> +#define AO_REMAP_REG10x4
> +
> +#define AO_CPU_CNTL  0x0
> + #define AO_CPU_CNTL_MEM_ADDR_UPPER  GENMASK(28, 16)
> + #define AO_CPU_CNTL_HALTBIT(9)
> + #define AO_CPU_CNTL_UNKNONWNBIT(8)
> + #define AO_CPU_CNTL_RUN BIT(0)
> +
> +#define AO_CPU_STAT  0x4
> +
> +#define AO_SECURE_REG0   0x0
> + #define AO_SECURE_REG0_UNKNOWN  GENMASK(23, 8)
> +
> +#define MESON_AO_RPROC_SRAM_USABLE_BITS  GENMASK(31, 20)
> +#define MESON_AO_RPROC_MEMORY_OFFSET 0x1000
> +
> +struct meson_mx_ao_arc_rproc_priv {
> + void __iomem*remap_base;
> + void __iomem*cpu_base;
> + unsigned long   sram_va;
> + phys_addr_t sram_pa;
> + size_t  sram_size;
> + struct gen_pool *sram_pool;
> + struct reset_control*arc_reset;
> + struct clk  *arc_pclk;
> + struct regmap   

<    1   2   3   4   5   6   7   8   9   10   >