Re: [PATCH] tracing/synthetic: Print out u64 values properly

2023-09-15 Thread Tero Kristo

Hi Masami,

On 15/09/2023 09:01, Masami Hiramatsu (Google) wrote:

Hi Tero,

On Mon, 11 Sep 2023 17:17:04 +0300
Tero Kristo  wrote:


The synth traces incorrectly print pointer to the synthetic event values
instead of the actual value when using u64 type. Fix by addressing the
contents of the union properly.

Thanks for pointing it out.
But I would like to see a new "case 8:" print code instead of changing
"default". Can you keep the default as it is and add "case 8:" case there?


Are you sure about that? I think keeping the default as is would just 
print out a useless pointer value to the synth event itself (which is 
what happened with u64 type.)


Anyways, that requires a new patch to be created on top as this has hit 
the mainline as a fix already.


-Tero




Thanks,


Fixes: ddeea494a16f ("tracing/synthetic: Use union instead of casts")
Cc: sta...@vger.kernel.org
Signed-off-by: Tero Kristo 
---
  kernel/trace/trace_events_synth.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace_events_synth.c 
b/kernel/trace/trace_events_synth.c
index 7fff8235075f..070365959c0a 100644
--- a/kernel/trace/trace_events_synth.c
+++ b/kernel/trace/trace_events_synth.c
@@ -337,7 +337,7 @@ static void print_synth_event_num_val(struct trace_seq *s,
break;
  
  	default:

-   trace_seq_printf(s, print_fmt, name, val, space);
+   trace_seq_printf(s, print_fmt, name, val->as_u64, space);
break;
}
  }
--
2.40.1





[PATCH] tracing/synthetic: Print out u64 values properly

2023-09-11 Thread Tero Kristo
The synth traces incorrectly print pointer to the synthetic event values
instead of the actual value when using u64 type. Fix by addressing the
contents of the union properly.

Fixes: ddeea494a16f ("tracing/synthetic: Use union instead of casts")
Cc: sta...@vger.kernel.org
Signed-off-by: Tero Kristo 
---
 kernel/trace/trace_events_synth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace_events_synth.c 
b/kernel/trace/trace_events_synth.c
index 7fff8235075f..070365959c0a 100644
--- a/kernel/trace/trace_events_synth.c
+++ b/kernel/trace/trace_events_synth.c
@@ -337,7 +337,7 @@ static void print_synth_event_num_val(struct trace_seq *s,
break;
 
default:
-   trace_seq_printf(s, print_fmt, name, val, space);
+   trace_seq_printf(s, print_fmt, name, val->as_u64, space);
break;
}
 }
-- 
2.40.1



Re: [PATCH v5 5/5] clk: ti: add am33xx/am43xx spread spectrum clock support

2021-04-19 Thread Tero Kristo

On 18/04/2021 17:56, Dario Binacchi wrote:

The patch enables spread spectrum clocking (SSC) for MPU and LCD PLLs.
As reported by the TI spruh73x/spruhl7x RM, SSC is only supported for
the DISP/LCD and MPU PLLs on am33xx/am43xx. SSC is not supported for
DDR, PER, and CORE PLLs.

Calculating the required values and setting the registers accordingly
was taken from the set_mpu_spreadspectrum routine contained in the
arch/arm/mach-omap2/am33xx/clock_am33xx.c file of the u-boot project.

In locked condition, DPLL output clock = CLKINP *[M/N]. In case of
SSC enabled, the reference manual explains that there is a restriction
of range of M values. Since the omap2_dpll_round_rate routine attempts
to select the minimum possible N, the value of M obtained is not
guaranteed to be within the range required. With the new "ti,min-div"
parameter it is possible to increase N and consequently M to satisfy the
constraint imposed by SSC.

Signed-off-by: Dario Binacchi 


Reviewed-by: Tero Kristo 



---

Changes in v5:
- Remove ssc_ack_mask field from dpll_data structure. It was not used.
- Change ssc_downspread type from u8 to bool in dpll_data structure.

Changes in v4:
- Update commit message.

Changes in v3:
- Use "ti,ssc-modfreq-hz" binding instead of "ti,ssc-modfreq".

Changes in v2:
- Move the DT changes to the previous patch in the series.

  drivers/clk/ti/dpll.c | 39 ++
  drivers/clk/ti/dpll3xxx.c | 85 +++
  include/linux/clk/ti.h| 22 ++
  3 files changed, 146 insertions(+)

diff --git a/drivers/clk/ti/dpll.c b/drivers/clk/ti/dpll.c
index d6f1ac5b53e1..e9f9aee936ae 100644
--- a/drivers/clk/ti/dpll.c
+++ b/drivers/clk/ti/dpll.c
@@ -290,7 +290,9 @@ static void __init of_ti_dpll_setup(struct device_node 
*node,
struct clk_init_data *init = NULL;
const char **parent_names = NULL;
struct dpll_data *dd = NULL;
+   int ssc_clk_index;
u8 dpll_mode = 0;
+   u32 min_div;
  
  	dd = kmemdup(ddt, sizeof(*dd), GFP_KERNEL);

clk_hw = kzalloc(sizeof(*clk_hw), GFP_KERNEL);
@@ -345,6 +347,27 @@ static void __init of_ti_dpll_setup(struct device_node 
*node,
if (dd->autoidle_mask) {
if (ti_clk_get_reg_addr(node, 3, >autoidle_reg))
goto cleanup;
+
+   ssc_clk_index = 4;
+   } else {
+   ssc_clk_index = 3;
+   }
+
+   if (dd->ssc_deltam_int_mask && dd->ssc_deltam_frac_mask &&
+   dd->ssc_modfreq_mant_mask && dd->ssc_modfreq_exp_mask) {
+   if (ti_clk_get_reg_addr(node, ssc_clk_index++,
+   >ssc_deltam_reg))
+   goto cleanup;
+
+   if (ti_clk_get_reg_addr(node, ssc_clk_index++,
+   >ssc_modfreq_reg))
+   goto cleanup;
+
+   of_property_read_u32(node, "ti,ssc-modfreq-hz",
+>ssc_modfreq);
+   of_property_read_u32(node, "ti,ssc-deltam", >ssc_deltam);
+   dd->ssc_downspread =
+   of_property_read_bool(node, "ti,ssc-downspread");
}
  
  	if (of_property_read_bool(node, "ti,low-power-stop"))

@@ -356,6 +379,10 @@ static void __init of_ti_dpll_setup(struct device_node 
*node,
if (of_property_read_bool(node, "ti,lock"))
dpll_mode |= 1 << DPLL_LOCKED;
  
+	if (!of_property_read_u32(node, "ti,min-div", _div) &&

+   min_div > dd->min_divider)
+   dd->min_divider = min_div;
+
if (dpll_mode)
dd->modes = dpll_mode;
  
@@ -585,8 +612,14 @@ static void __init of_ti_am3_no_gate_dpll_setup(struct device_node *node)

const struct dpll_data dd = {
.idlest_mask = 0x1,
.enable_mask = 0x7,
+   .ssc_enable_mask = 0x1 << 12,
+   .ssc_downspread_mask = 0x1 << 14,
.mult_mask = 0x7ff << 8,
.div1_mask = 0x7f,
+   .ssc_deltam_int_mask = 0x3 << 18,
+   .ssc_deltam_frac_mask = 0x3,
+   .ssc_modfreq_mant_mask = 0x7f,
+   .ssc_modfreq_exp_mask = 0x7 << 8,
.max_multiplier = 2047,
.max_divider = 128,
.min_divider = 1,
@@ -645,8 +678,14 @@ static void __init of_ti_am3_dpll_setup(struct device_node 
*node)
const struct dpll_data dd = {
.idlest_mask = 0x1,
.enable_mask = 0x7,
+   .ssc_enable_mask = 0x1 << 12,
+   .ssc_downspread_mask = 0x1 << 14,
.mult_mask = 0x7ff << 8,
.div1_mask = 0x7f,
+   .ssc_deltam_int_mask = 0x3 << 18,
+   .ssc_de

Re: [PATCH v4 5/5] clk: ti: add am33xx/am43xx spread spectrum clock support

2021-04-16 Thread Tero Kristo

Hi Dario,

Spent some time looking at this, had to read through the TRM chapter of 
it also in quite detailed level to figure out how this is supposed to 
work out.


Other than couple of minor nits below, the code seems ok to me. What is 
the testing that has been done with this?


On 01/04/2021 22:37, Dario Binacchi wrote:

The patch enables spread spectrum clocking (SSC) for MPU and LCD PLLs.
As reported by the TI spruh73x/spruhl7x RM, SSC is only supported for
the DISP/LCD and MPU PLLs on am33xx/am43xx. SSC is not supported for
DDR, PER, and CORE PLLs.

Calculating the required values and setting the registers accordingly
was taken from the set_mpu_spreadspectrum routine contained in the
arch/arm/mach-omap2/am33xx/clock_am33xx.c file of the u-boot project.

In locked condition, DPLL output clock = CLKINP *[M/N]. In case of
SSC enabled, the reference manual explains that there is a restriction
of range of M values. Since the omap2_dpll_round_rate routine attempts
to select the minimum possible N, the value of M obtained is not
guaranteed to be within the range required. With the new "ti,min-div"
parameter it is possible to increase N and consequently M to satisfy the
constraint imposed by SSC.

Signed-off-by: Dario Binacchi 

---





/* REVISIT: Set ramp-up delay? */
diff --git a/include/linux/clk/ti.h b/include/linux/clk/ti.h
index c62f6fa6763d..cba093de62d8 100644
--- a/include/linux/clk/ti.h
+++ b/include/linux/clk/ti.h
@@ -63,6 +63,18 @@ struct clk_omap_reg {
   * @auto_recal_bit: bitshift of the driftguard enable bit in @control_reg
   * @recal_en_bit: bitshift of the PRM_IRQENABLE_* bit for recalibration IRQs
   * @recal_st_bit: bitshift of the PRM_IRQSTATUS_* bit for recalibration IRQs
+ * @ssc_deltam_reg: register containing the DPLL SSC frequency spreading
+ * @ssc_modfreq_reg: register containing the DPLL SSC modulation frequency
+ * @ssc_modfreq_mant_mask: mask of the mantissa component in @ssc_modfreq_reg
+ * @ssc_modfreq_exp_mask: mask of the exponent component in @ssc_modfreq_reg
+ * @ssc_enable_mask: mask of the DPLL SSC enable bit in @control_reg
+ * @ssc_ack_mask: mask of the DPLL SSC turned on/off bit in @control_reg
+ * @ssc_downspread_mask: mask of the DPLL SSC low frequency only bit in
+ *   @control_reg
+ * @ssc_modfreq: the DPLL SSC frequency modulation in kHz
+ * @ssc_deltam: the DPLL SSC frequency spreading in permille (10th of percent)
+ * @ssc_downspread: require the only low frequency spread of the DPLL in SSC
+ *   mode
   * @flags: DPLL type/features (see below)
   *
   * Possible values for @flags:
@@ -110,6 +122,18 @@ struct dpll_data {
u8  auto_recal_bit;
u8  recal_en_bit;
u8  recal_st_bit;
+   struct clk_omap_reg ssc_deltam_reg;
+   struct clk_omap_reg ssc_modfreq_reg;
+   u32 ssc_deltam_int_mask;
+   u32 ssc_deltam_frac_mask;
+   u32 ssc_modfreq_mant_mask;
+   u32 ssc_modfreq_exp_mask;
+   u32 ssc_enable_mask;
+   u32 ssc_ack_mask;


ssc_ack_mask is not used for anything in the code.


+   u32 ssc_downspread_mask;
+   u32 ssc_modfreq;
+   u32 ssc_deltam;
+   u8  ssc_downspread;


ssc_downspread should be boolean?


u8  flags;
  };
  





Re: [PATCH 0/4] dt-bindings: soc/arm: Convert pending ti,sci* bindings to json format

2021-04-16 Thread Tero Kristo

On 16/04/2021 09:37, Nishanth Menon wrote:

Hi,

I understand that the following series belong to various maintainers,
but, it is a bit better reviewed as a single series for
cohesiveness.

There are also dts fixups that this series exposes, which is good, but
I chose to hold them back for now pending binding review at least. The
complete series is available in [1] if folks are curious.

Nishanth Menon (4):
   dt-bindings: reset: Convert ti,sci-reset to json schema
   dt-bindings: clock: Convert ti,sci-clk to json schema
   dt-bindings: soc: ti: Convert ti,sci-pm-domain to json schema
   dt-bindings: arm: keystone: Convert ti,sci to json schema


For the whole series:

Reviewed-by: Tero Kristo 



  .../bindings/arm/keystone/ti,sci.txt  |  86 
  .../bindings/arm/keystone/ti,sci.yaml | 129 ++
  .../devicetree/bindings/clock/ti,sci-clk.txt  |  36 -
  .../devicetree/bindings/clock/ti,sci-clk.yaml |  52 +++
  .../bindings/reset/ti,sci-reset.txt   |  62 -
  .../bindings/reset/ti,sci-reset.yaml  |  51 +++
  .../bindings/soc/ti/sci-pm-domain.txt |  65 -
  .../bindings/soc/ti/sci-pm-domain.yaml|  59 
  8 files changed, 291 insertions(+), 249 deletions(-)
  delete mode 100644 Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
  create mode 100644 Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
  delete mode 100644 Documentation/devicetree/bindings/clock/ti,sci-clk.txt
  create mode 100644 Documentation/devicetree/bindings/clock/ti,sci-clk.yaml
  delete mode 100644 Documentation/devicetree/bindings/reset/ti,sci-reset.txt
  create mode 100644 Documentation/devicetree/bindings/reset/ti,sci-reset.yaml
  delete mode 100644 Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
  create mode 100644 Documentation/devicetree/bindings/soc/ti/sci-pm-domain.yaml

[1] https://github.com/nmenon/linux-2.6-playground/commits/yaml/tisci

Regards,
Nishanth Menon





Re: [PATCH 0/2] fdt: translate address if #size-cells = <0>

2021-04-12 Thread Tero Kristo

On 11/04/2021 22:30, Dario Binacchi wrote:



Il 09/04/2021 12:32 Tero Kristo  ha scritto:

  
On 08/04/2021 23:24, Dario Binacchi wrote:



Il 07/04/2021 15:21 Tero Kristo  ha scritto:

   
On 07/04/2021 15:52, Rob Herring wrote:

On Wed, Apr 7, 2021 at 2:07 AM Dario Binacchi  wrote:




Il 07/04/2021 03:16 Rob Herring  ha scritto:


On Tue, Apr 6, 2021 at 5:02 PM Dario Binacchi  wrote:




Il 06/04/2021 16:06 Rob Herring  ha scritto:


On Fri, Apr 2, 2021 at 2:21 PM Dario Binacchi  wrote:



The series comes from my commit in U-boot
d64b9cdcd4 ("fdt: translate address if #size-cells = <0>")
and from the subsequent exchange of emails at the end of which I was
suggested to send the patch to the linux kernel
(https://patchwork.ozlabs.org/project/uboot/patch/1614324949-61314-1-git-send-email-bmeng...@gmail.com/).


It's 'ranges' that determines translatable which is missing from the
DT. This should have not had a 0 size either though maybe we could
support that.


I have replied to the email you sent to the u-boot mailing list



Does the DT have to be updated anyways for your spread spectrum support?


The spread spectrum support patch does not need this patch to work. They belong
to two different series.


That's not what I asked. Is the spread spectrum support forcing a DT
update for users?


Yes, the deltam and modfreq registers must be added to the DPLL clocks.


That's a shame given this dts has been mostly untouched since 2013.



I think technically it would be possible to map these registers within
the driver also, seeing there are like a handful of the DPLLs for both
am3/am4 which are impacted. Just add a new compatible or something, or
alternatively parse the register addresses and populate the
deltam/modfreq registers based on that.


I have not added new compatibles, but I have added the offset of the delta and 
modfreq
registers to the data structures used by the DPLL drivers and I have set them 
in the
related setup functions.
https://lore.kernel.org/patchwork/patch/1406590/


True, I just said that technically it would be possible to add this data
within the driver itself to avoid modifying DT if that would be preferred.


In the review of the series no one asked not to change the device tree but it 
is also true
that no review has been made on the patch 'clk: ti: add am33xx / am43xx spread 
spectrum clock support',
the one to be applied on the drivers that support the SSC.
I take this opportunity to ask you if you can kindly review that patch.


The clock driver patch itself seems fine, the devil is on the DT side, 
and how we are going to re-arrange the DT data to accommodate it.











If the DT has to be changed anyways (not really
great policy), then you could fix this in the DT at the same time.


I could put the fix to the device tree in that series, although I wouldn't
create a single patch to fix and add the SSC registers. First the size-cells = 
<0>
fix patch and then the SSC patch.
Do you agree?


By at the same time, I really just meant within 1 release.

But I'd like to hear TI maintainers' thoughts on this.


I did post a comment on patch #1 questioning the approach from TI clock
driver perspective, imho I can't see why these two patches would be
needed right now.


Fix to above, it was patch #2 I was referring to.



Because U-boot maintainers asked me after I sent them my patch on this issue.
I believe that the email exchange that took place in the U-boot 
(https://patchwork.ozlabs.org/project/uboot/patch/1614324949-61314-1-git-send-email-bmeng...@gmail.com/)
and Linux kernel mailing lists showed that:
- The patch 'fdt: translate address if # size-cells = <0>' is wrong (U-boot has 
accepted
it, and it will have to be reverted).
- However, the same patch highlighted that it is wrong to use the size-cells = 
<0> property
in the prcm_clocks and scm_clocks nodes of device tree.
- Rob agrees that in the case of the am3xx this is the right choice:


Well, I don't quite like where this is ending at. Basically you are just
modifying the am3/am4 DTs adding a size spec for every clock node. This
leaves the omap3/omap4/omap5/dra7 in the old format. Should someone
convert those also? Has anybody tested what happens with the u-boot
change on those platforms? Or with the kernel change proposed for the TI
clock driver?

Also, none of this changes the fact that imho patch #2 in this series is
wrong and should be fixed. Doing ioremap for every clock node is at
least costly (dra7xx has some 180 clock nodes based on quick grep) and
also potentially breaks things (you get extremely fragmented iomappings,
and some of them might even get rejected because you end up in the same
4K page), and should be avoided.


You are right, and in fact in my previous email, I proposed only to change the
ti_clk_get_reg_addr() from:
- if (of_property_read_u32_index (node, "reg", index, & val)) {
+ if (of_property_read_u32_index (node, "reg", in

Re: [PATCH 0/2] fdt: translate address if #size-cells = <0>

2021-04-09 Thread Tero Kristo

On 08/04/2021 23:24, Dario Binacchi wrote:



Il 07/04/2021 15:21 Tero Kristo  ha scritto:

  
On 07/04/2021 15:52, Rob Herring wrote:

On Wed, Apr 7, 2021 at 2:07 AM Dario Binacchi  wrote:




Il 07/04/2021 03:16 Rob Herring  ha scritto:


On Tue, Apr 6, 2021 at 5:02 PM Dario Binacchi  wrote:




Il 06/04/2021 16:06 Rob Herring  ha scritto:


On Fri, Apr 2, 2021 at 2:21 PM Dario Binacchi  wrote:



The series comes from my commit in U-boot
d64b9cdcd4 ("fdt: translate address if #size-cells = <0>")
and from the subsequent exchange of emails at the end of which I was
suggested to send the patch to the linux kernel
(https://patchwork.ozlabs.org/project/uboot/patch/1614324949-61314-1-git-send-email-bmeng...@gmail.com/).


It's 'ranges' that determines translatable which is missing from the
DT. This should have not had a 0 size either though maybe we could
support that.


I have replied to the email you sent to the u-boot mailing list



Does the DT have to be updated anyways for your spread spectrum support?


The spread spectrum support patch does not need this patch to work. They belong
to two different series.


That's not what I asked. Is the spread spectrum support forcing a DT
update for users?


Yes, the deltam and modfreq registers must be added to the DPLL clocks.


That's a shame given this dts has been mostly untouched since 2013.



I think technically it would be possible to map these registers within
the driver also, seeing there are like a handful of the DPLLs for both
am3/am4 which are impacted. Just add a new compatible or something, or
alternatively parse the register addresses and populate the
deltam/modfreq registers based on that.


I have not added new compatibles, but I have added the offset of the delta and 
modfreq
registers to the data structures used by the DPLL drivers and I have set them 
in the
related setup functions.
https://lore.kernel.org/patchwork/patch/1406590/


True, I just said that technically it would be possible to add this data 
within the driver itself to avoid modifying DT if that would be preferred.







If the DT has to be changed anyways (not really
great policy), then you could fix this in the DT at the same time.


I could put the fix to the device tree in that series, although I wouldn't
create a single patch to fix and add the SSC registers. First the size-cells = 
<0>
fix patch and then the SSC patch.
Do you agree?


By at the same time, I really just meant within 1 release.

But I'd like to hear TI maintainers' thoughts on this.


I did post a comment on patch #1 questioning the approach from TI clock
driver perspective, imho I can't see why these two patches would be
needed right now.


Fix to above, it was patch #2 I was referring to.



Because U-boot maintainers asked me after I sent them my patch on this issue.
I believe that the email exchange that took place in the U-boot 
(https://patchwork.ozlabs.org/project/uboot/patch/1614324949-61314-1-git-send-email-bmeng...@gmail.com/)
and Linux kernel mailing lists showed that:
- The patch 'fdt: translate address if # size-cells = <0>' is wrong (U-boot has 
accepted
   it, and it will have to be reverted).
- However, the same patch highlighted that it is wrong to use the size-cells = 
<0> property
   in the prcm_clocks and scm_clocks nodes of device tree.
- Rob agrees that in the case of the am3xx this is the right choice:


Well, I don't quite like where this is ending at. Basically you are just 
modifying the am3/am4 DTs adding a size spec for every clock node. This 
leaves the omap3/omap4/omap5/dra7 in the old format. Should someone 
convert those also? Has anybody tested what happens with the u-boot 
change on those platforms? Or with the kernel change proposed for the TI 
clock driver?


Also, none of this changes the fact that imho patch #2 in this series is 
wrong and should be fixed. Doing ioremap for every clock node is at 
least costly (dra7xx has some 180 clock nodes based on quick grep) and 
also potentially breaks things (you get extremely fragmented iomappings, 
and some of them might even get rejected because you end up in the same 
4K page), and should be avoided.


If things would be fixed properly, we would get rid of the clock nodes 
from the DT completely and just leave the clock providers in place; 
clocks would be specified via something like 'clocks = < 
AM3_ADC_TSC_FCK>;' similar to what is done with the clkctrl entries, and 
rest of the clock data would be built-in to the clock driver. This would 
completely get rid of any future compatibility issues and the need to 
tweak the DT if some clock driver would need modifications to support 
some new feature.


-Tero


diff --git a/arch/arm/boot/dts/am33xx-l4.dtsi b/arch/arm/boot/dts/am33xx-l4.dtsi
index 1fb22088caeb..59b0a0cf211e 100644
--- a/arch/arm/boot/dts/am33xx-l4.dtsi
+++ b/arch/arm/boot/dts/am33xx-l4.dtsi
@@ -110,7 +110,8 @@
  

Re: [PATCH 0/2] fdt: translate address if #size-cells = <0>

2021-04-07 Thread Tero Kristo

On 07/04/2021 15:52, Rob Herring wrote:

On Wed, Apr 7, 2021 at 2:07 AM Dario Binacchi  wrote:




Il 07/04/2021 03:16 Rob Herring  ha scritto:


On Tue, Apr 6, 2021 at 5:02 PM Dario Binacchi  wrote:




Il 06/04/2021 16:06 Rob Herring  ha scritto:


On Fri, Apr 2, 2021 at 2:21 PM Dario Binacchi  wrote:



The series comes from my commit in U-boot
d64b9cdcd4 ("fdt: translate address if #size-cells = <0>")
and from the subsequent exchange of emails at the end of which I was
suggested to send the patch to the linux kernel
(https://patchwork.ozlabs.org/project/uboot/patch/1614324949-61314-1-git-send-email-bmeng...@gmail.com/).


It's 'ranges' that determines translatable which is missing from the
DT. This should have not had a 0 size either though maybe we could
support that.


I have replied to the email you sent to the u-boot mailing list



Does the DT have to be updated anyways for your spread spectrum support?


The spread spectrum support patch does not need this patch to work. They belong
to two different series.


That's not what I asked. Is the spread spectrum support forcing a DT
update for users?


Yes, the deltam and modfreq registers must be added to the DPLL clocks.


That's a shame given this dts has been mostly untouched since 2013.



I think technically it would be possible to map these registers within 
the driver also, seeing there are like a handful of the DPLLs for both 
am3/am4 which are impacted. Just add a new compatible or something, or 
alternatively parse the register addresses and populate the 
deltam/modfreq registers based on that.



If the DT has to be changed anyways (not really
great policy), then you could fix this in the DT at the same time.


I could put the fix to the device tree in that series, although I wouldn't
create a single patch to fix and add the SSC registers. First the size-cells = 
<0>
fix patch and then the SSC patch.
Do you agree?


By at the same time, I really just meant within 1 release.

But I'd like to hear TI maintainers' thoughts on this.


I did post a comment on patch #1 questioning the approach from TI clock 
driver perspective, imho I can't see why these two patches would be 
needed right now.


-Tero


Re: [PATCH 2/2] clk: ti: get register address from device tree

2021-04-06 Thread Tero Kristo

On 02/04/2021 22:20, Dario Binacchi wrote:

Until now, only the register offset was retrieved from the device tree
to be added, during access, to a common base address for the clocks.
If possible, we try to retrieve the physical address of the register
directly from the device tree.


The physical address is derived from the base address of the clock 
provider, it is not derived from the clock node itself.


Doing what this patch does may actually break things, as you end up 
creating an individual ioremap for every single clock register, and they 
are typically a word apart from each other. In the TI clock driver case, 
the ioremap is done only once for the whole clock register space.


-Tero



Signed-off-by: Dario Binacchi 

---

  drivers/clk/ti/clk.c | 13 -
  1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/ti/clk.c b/drivers/clk/ti/clk.c
index 3da33c786d77..938f5a2cb425 100644
--- a/drivers/clk/ti/clk.c
+++ b/drivers/clk/ti/clk.c
@@ -265,9 +265,21 @@ int __init ti_clk_retry_init(struct device_node *node, 
void *user,
  int ti_clk_get_reg_addr(struct device_node *node, int index,
struct clk_omap_reg *reg)
  {
+   const __be32 *addrp;
+   u64 size, addr = OF_BAD_ADDR;
+   unsigned int flags;
u32 val;
int i;
  
+	addrp = of_get_address(node, index, , );

+   if (addrp)
+   addr = of_translate_address(node, addrp);
+
+   if (addr != OF_BAD_ADDR) {
+   reg->ptr = ioremap(addr, sizeof(u32));
+   return 0;
+   }
+
for (i = 0; i < CLK_MAX_MEMMAPS; i++) {
if (clocks_node_ptr[i] == node->parent)
break;
@@ -287,7 +299,6 @@ int ti_clk_get_reg_addr(struct device_node *node, int index,
  
  	reg->offset = val;

reg->ptr = NULL;
-
return 0;
  }
  





Re: [PATCH 00/21] [Set 2] Rid W=1 warnings from Clock

2021-02-07 Thread Tero Kristo

On 26/01/2021 14:45, Lee Jones wrote:

This set is part of a larger effort attempting to clean-up W=1
kernel builds, which are currently overwhelmingly riddled with
niggly little warnings.

This is the last set.  Clock is clean after this.

Lee Jones (21):
   clk: zynq: pll: Fix kernel-doc formatting in 'clk_register_zynq_pll's
 header
   clk: ti: clkt_dpll: Fix some kernel-doc misdemeanours
   clk: ti: dpll3xxx: Fix some kernel-doc headers and promote other
 worthy ones
   clk: qcom: clk-regmap: Provide missing description for
 'devm_clk_register_regmap()'s dev param
   clk: sunxi: clk-sun9i-core: Demote non-conformant kernel-doc headers
   clk: sunxi: clk-usb: Demote obvious kernel-doc abuse
   clk: tegra: clk-tegra30: Remove unused variable 'reg'
   clk: clkdev: Ignore suggestion to use gnu_printf() as it's not
 appropriate here
   clk: tegra: cvb: Provide missing description for
 'tegra_cvb_add_opp_table()'s align param
   clk: ti: dpll44xx: Fix some potential doc-rot
   clk: renesas: renesas-cpg-mssr: Fix formatting issues for
 'smstpcr_saved's documentation
   clk: sunxi: clk-sun6i-ar100: Demote non-conformant kernel-doc header
   clk: qcom: gcc-ipq4019: Remove unused variable 'ret'
   clk: clk-fixed-mmio: Demote obvious kernel-doc abuse
   clk: clk-npcm7xx: Remove unused static const tables 'npcm7xx_gates'
 and 'npcm7xx_divs_fx'
   clk: qcom: mmcc-msm8974: Remove unused static const tables
 'mmcc_xo_mmpll0_1_2_gpll0{map}'
   clk: clk-xgene: Add description for 'mask' and fix formatting for
 'flags'
   clk: qcom: clk-rpm: Remove a bunch of superfluous code
   clk: spear: Move prototype to accessible header
   clk: imx: Move 'imx6sl_set_wait_clk()'s prototype out to accessible
 header
   clk: zynqmp: divider: Add missing description for 'max_div'

  arch/arm/mach-imx/common.h |   1 -
  arch/arm/mach-imx/cpuidle-imx6sl.c |   1 +
  arch/arm/mach-imx/pm-imx6.c|   1 +
  arch/arm/mach-spear/generic.h  |  12 ---
  arch/arm/mach-spear/spear13xx.c|   1 +
  drivers/clk/clk-fixed-mmio.c   |   2 +-
  drivers/clk/clk-npcm7xx.c  | 108 -
  drivers/clk/clk-xgene.c|   5 +-
  drivers/clk/clkdev.c   |   7 ++
  drivers/clk/imx/clk-imx6sl.c   |   1 +
  drivers/clk/qcom/clk-regmap.c  |   1 +
  drivers/clk/qcom/clk-rpm.c |  63 ---
  drivers/clk/qcom/gcc-ipq4019.c |   7 +-
  drivers/clk/qcom/mmcc-msm8974.c|  16 
  drivers/clk/renesas/renesas-cpg-mssr.c |   4 +-
  drivers/clk/spear/spear1310_clock.c|   1 +
  drivers/clk/spear/spear1340_clock.c|   1 +
  drivers/clk/sunxi/clk-sun6i-ar100.c|   2 +-
  drivers/clk/sunxi/clk-sun9i-core.c |   8 +-
  drivers/clk/sunxi/clk-usb.c|   2 +-
  drivers/clk/tegra/clk-tegra30.c|   5 +-
  drivers/clk/tegra/cvb.c|   1 +
  drivers/clk/ti/clkt_dpll.c |   3 +-
  drivers/clk/ti/dpll3xxx.c  |  20 ++---
  drivers/clk/ti/dpll44xx.c  |   6 +-


For the TI portions:

Reviewed-by: Tero Kristo 


  drivers/clk/zynq/pll.c |  12 +--
  drivers/clk/zynqmp/divider.c   |   1 +
  include/linux/clk/imx.h|  15 
  include/linux/clk/spear.h  |  23 ++
  29 files changed, 92 insertions(+), 238 deletions(-)
  create mode 100644 include/linux/clk/imx.h
  create mode 100644 include/linux/clk/spear.h

Cc: Ahmad Fatoum 
Cc: Andy Gross 
Cc: Avi Fishman 
Cc: Benjamin Fair 
Cc: Bjorn Andersson 
Cc: Boris BREZILLON 
Cc: Chen-Yu Tsai 
Cc: "Emilio López" 
Cc: Fabio Estevam 
Cc: Geert Uytterhoeven 
Cc: Jan Kotas 
Cc: Jernej Skrabec 
Cc: Jonathan Hunter 
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-arm-...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: linux-o...@vger.kernel.org
Cc: linux-renesas-...@vger.kernel.org
Cc: linux-te...@vger.kernel.org
Cc: Loc Ho 
Cc: Maxime Ripard 
Cc: Michael Turquette 
Cc: Michal Simek 
Cc: Nancy Yuen 
Cc: Nuvoton Technologies 
Cc: NXP Linux Team 
Cc: open...@lists.ozlabs.org
Cc: Patrick Venture 
Cc: Pengutronix Kernel Team 
Cc: Peter De Schrijver 
Cc: Philipp Zabel 
Cc: Prashant Gaikwad 
Cc: Rajan Vaja 
Cc: Rajeev Kumar 
Cc: Richard Woodruff 
Cc: Russell King 
Cc: Sascha Hauer 
Cc: Shawn Guo 
Cc: Shiraz Hashim 
Cc: "Sören Brinkmann" 
Cc: Stephen Boyd 
Cc: Tali Perry 
Cc: Tero Kristo 
Cc: Thierry Reding 
Cc: Tomer Maimon 
Cc: Viresh Kumar 





Re: [PATCH] arm64: dts: ti: k3*: Fixup PMU compatibility to be CPU specific

2021-01-21 Thread Tero Kristo

On 20/01/2021 21:51, Nishanth Menon wrote:

We can use CPU specific pmu configuration to expose the appropriate
CPU specific events rather than just the basic generic pmuv3 perf
events.

Reported-by: Sudeep Holla 
Signed-off-by: Nishanth Menon 


Reviewed-by: Tero Kristo 


---

AM65: https://pastebin.ubuntu.com/p/TF2cCMySkt/
J721E: https://pastebin.ubuntu.com/p/jgGPNmNgG3/
J7200: https://pastebin.ubuntu.com/p/Kfc3VHHXNB/

Original report: 
https://lore.kernel.org/linux-arm-kernel/20210119172412.smsjdo2sjzqi5vcn@bogus/

I have'nt split this patch up for fixes tag primarily because the
basic functionality works and this is an improvement than a critical
fixup to backport for older kernels.

  arch/arm64/boot/dts/ti/k3-am65.dtsi  | 2 +-
  arch/arm64/boot/dts/ti/k3-j7200.dtsi | 2 +-
  arch/arm64/boot/dts/ti/k3-j721e.dtsi | 2 +-
  3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-am65.dtsi 
b/arch/arm64/boot/dts/ti/k3-am65.dtsi
index d84c0bc05023..a9fc1af03f27 100644
--- a/arch/arm64/boot/dts/ti/k3-am65.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65.dtsi
@@ -56,7 +56,7 @@ a53_timer0: timer-cl0-cpu0 {
};
  
  	pmu: pmu {

-   compatible = "arm,armv8-pmuv3";
+   compatible = "arm,cortex-a53-pmu";
/* Recommendation from GIC500 TRM Table A.3 */
interrupts = ;
};
diff --git a/arch/arm64/boot/dts/ti/k3-j7200.dtsi 
b/arch/arm64/boot/dts/ti/k3-j7200.dtsi
index 66169bcf7c9a..b7005b803149 100644
--- a/arch/arm64/boot/dts/ti/k3-j7200.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j7200.dtsi
@@ -114,7 +114,7 @@ a72_timer0: timer-cl0-cpu0 {
};
  
  	pmu: pmu {

-   compatible = "arm,armv8-pmuv3";
+   compatible = "arm,cortex-a72-pmu";
interrupts = ;
};
  
diff --git a/arch/arm64/boot/dts/ti/k3-j721e.dtsi b/arch/arm64/boot/dts/ti/k3-j721e.dtsi

index cc483f7344af..f0587fde147e 100644
--- a/arch/arm64/boot/dts/ti/k3-j721e.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721e.dtsi
@@ -115,7 +115,7 @@ a72_timer0: timer-cl0-cpu0 {
};
  
  	pmu: pmu {

-   compatible = "arm,armv8-pmuv3";
+   compatible = "arm,cortex-a72-pmu";
/* Recommendation from GIC500 TRM Table A.3 */
interrupts = ;
};





Re: [PATCH 16/20] clk: ti: gate: Fix possible doc-rot in 'omap36xx_gate_clk_enable_with_hsdiv_restore'

2021-01-20 Thread Tero Kristo

On 20/01/2021 11:30, Lee Jones wrote:

Fixes the following W=1 kernel build warning(s):

  drivers/clk/ti/gate.c:67: warning: Function parameter or member 'hw' not 
described in 'omap36xx_gate_clk_enable_with_hsdiv_restore'
  drivers/clk/ti/gate.c:67: warning: Excess function parameter 'clk' 
description in 'omap36xx_gate_clk_enable_with_hsdiv_restore'

Cc: Tero Kristo 
Cc: Michael Turquette 
Cc: Stephen Boyd 
Cc: linux-o...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Signed-off-by: Lee Jones 


Reviewed-by: Tero Kristo 


---
  drivers/clk/ti/gate.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/ti/gate.c b/drivers/clk/ti/gate.c
index 42389558418c5..b1d0fdb40a75a 100644
--- a/drivers/clk/ti/gate.c
+++ b/drivers/clk/ti/gate.c
@@ -55,7 +55,7 @@ static const struct clk_ops omap_gate_clk_hsdiv_restore_ops = 
{
  /**
   * omap36xx_gate_clk_enable_with_hsdiv_restore - enable clocks suffering
   * from HSDivider PWRDN problem Implements Errata ID: i556.
- * @clk: DPLL output struct clk
+ * @hw: DPLL output struct clk_hw
   *
   * 3630 only: dpll3_m3_ck, dpll4_m2_ck, dpll4_m3_ck, dpll4_m4_ck,
   * dpll4_m5_ck & dpll4_m6_ck dividers gets loaded with reset





Re: [PATCH 15/20] clk: ti: dpll: Fix misnaming of '_register_dpll()'s 'user' parameter

2021-01-20 Thread Tero Kristo

On 20/01/2021 11:30, Lee Jones wrote:

Fixes the following W=1 kernel build warning(s):

  drivers/clk/ti/dpll.c:163: warning: Function parameter or member 'user' not 
described in '_register_dpll'
  drivers/clk/ti/dpll.c:163: warning: Excess function parameter 'hw' 
description in '_register_dpll'

Cc: Tero Kristo 
Cc: Michael Turquette 
Cc: Stephen Boyd 
Cc: linux-o...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Signed-off-by: Lee Jones 


Reviewed-by: Tero Kristo 


---
  drivers/clk/ti/dpll.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/ti/dpll.c b/drivers/clk/ti/dpll.c
index 247510e306e2a..d6f1ac5b53e14 100644
--- a/drivers/clk/ti/dpll.c
+++ b/drivers/clk/ti/dpll.c
@@ -151,7 +151,7 @@ static const struct clk_ops dpll_x2_ck_ops = {
  
  /**

   * _register_dpll - low level registration of a DPLL clock
- * @hw: hardware clock definition for the clock
+ * @user: pointer to the hardware clock definition for the clock
   * @node: device node for the clock
   *
   * Finalizes DPLL registration process. In case a failure (clk-ref or





Re: [PATCH 13/20] clk: ti: clockdomain: Fix description for 'omap2_init_clk_clkdm's hw param

2021-01-20 Thread Tero Kristo

On 20/01/2021 11:30, Lee Jones wrote:

Fixes the following W=1 kernel build warning(s):

  drivers/clk/ti/clockdomain.c:107: warning: Function parameter or member 'hw' 
not described in 'omap2_init_clk_clkdm'
  drivers/clk/ti/clockdomain.c:107: warning: Excess function parameter 'clk' 
description in 'omap2_init_clk_clkdm'

Cc: Tero Kristo 
Cc: Michael Turquette 
Cc: Stephen Boyd 
Cc: linux-o...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Signed-off-by: Lee Jones 


Reviewed-by: Tero Kristo 


---
  drivers/clk/ti/clockdomain.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/ti/clockdomain.c b/drivers/clk/ti/clockdomain.c
index 700b7f44f6716..74831b2752b3b 100644
--- a/drivers/clk/ti/clockdomain.c
+++ b/drivers/clk/ti/clockdomain.c
@@ -97,7 +97,7 @@ void omap2_clkops_disable_clkdm(struct clk_hw *hw)
  
  /**

   * omap2_init_clk_clkdm - look up a clockdomain name, store pointer in clk
- * @clk: OMAP clock struct ptr to use
+ * @hw: Pointer to clk_hw_omap used to obtain OMAP clock struct ptr to use
   *
   * Convert a clockdomain name stored in a struct clk 'clk' into a
   * clockdomain pointer, and save it into the struct clk.  Intended to be





[PATCH] MAINTAINERS: Update my email address and maintainer level status

2020-12-17 Thread Tero Kristo
My employment with TI is ending tomorrow, so update the email address
entry in the maintainers file. Also, I don't expect to spend that much
time with maintaining TI code anymore, so downgrade the status level to
odd fixes only on areas where I remain as the main contact point for
now, and move myself as secondary contact point where someone else has
taken over the maintainership.

Cc: Stephen Boyd 
Cc: Michael Turquette 
Cc: Nishanth Menon 
Cc: Santosh Shilimkar 
Cc: Borislav Petkov 
Cc: Tony Luck 
Signed-off-by: Tero Kristo 
Signed-off-by: Tero Kristo 
---
 MAINTAINERS | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index f59ebd1eda3d..c362d8d9d316 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2615,8 +2615,8 @@ S:Maintained
 F: drivers/power/reset/keystone-reset.c
 
 ARM/TEXAS INSTRUMENTS K3 ARCHITECTURE
-M: Tero Kristo 
 M: Nishanth Menon 
+M: Tero Kristo 
 L: linux-arm-ker...@lists.infradead.org (moderated for non-subscribers)
 S: Supported
 F: Documentation/devicetree/bindings/arm/ti/k3.yaml
@@ -6465,9 +6465,9 @@ S:Maintained
 F: drivers/edac/skx_*.[ch]
 
 EDAC-TI
-M: Tero Kristo 
+M: Tero Kristo 
 L: linux-e...@vger.kernel.org
-S: Maintained
+S: Odd Fixes
 F: drivers/edac/ti_edac.c
 
 EDIROL UA-101/UA-1000 DRIVER
@@ -17503,7 +17503,7 @@ F:  drivers/iio/dac/ti-dac7612.c
 
 TEXAS INSTRUMENTS' SYSTEM CONTROL INTERFACE (TISCI) PROTOCOL DRIVER
 M: Nishanth Menon 
-M: Tero Kristo 
+M: Tero Kristo 
 M: Santosh Shilimkar 
 L: linux-arm-ker...@lists.infradead.org
 S: Maintained
@@ -17647,9 +17647,9 @@ S:  Maintained
 F: drivers/clk/clk-cdce706.c
 
 TI CLOCK DRIVER
-M: Tero Kristo 
+M: Tero Kristo 
 L: linux-o...@vger.kernel.org
-S: Maintained
+S: Odd Fixes
 F: drivers/clk/ti/
 F: include/linux/clk/ti.h
 
-- 
2.17.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH V2 3/5] arm64: dts: ti: am65/j721e: Fix up un-necessary status set to "okay" for crypto

2020-11-12 Thread Tero Kristo

On 12/11/2020 03:49, Nishanth Menon wrote:

The default state of a device tree node is "okay". There is no specific
use of explicitly adding status = "okay" in the SoC dtsi.

Fixes: 8ebcaaae8017 ("arm64: dts: ti: k3-j721e-main: Add crypto accelerator 
node")
Fixes: b366b2409c97 ("arm64: dts: ti: k3-am6: Add crypto accelarator node")
Signed-off-by: Nishanth Menon 
Cc: Keerthy 


Acked-by: Tero Kristo 


---
Changes since V1:
- No change.

V1: https://lore.kernel.org/linux-arm-kernel/20201104224356.18040-4...@ti.com/

  arch/arm64/boot/dts/ti/k3-am65-main.dtsi  | 1 -
  arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 2 --
  2 files changed, 3 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi 
b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
index 21e50021dd83..2bd66a9e4b1d 100644
--- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
@@ -119,7 +119,6 @@ crypto: crypto@4e0 {
#address-cells = <2>;
#size-cells = <2>;
ranges = <0x0 0x04e0 0x00 0x04e0 0x0 0x3>;
-   status = "okay";
  
  		dmas = <_udmap 0xc000>, <_udmap 0x4000>,

<_udmap 0x4001>;
diff --git a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi 
b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
index b54332d6fdc5..9747c387385b 100644
--- a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
@@ -345,8 +345,6 @@ main_crypto: crypto@4e0 {
#size-cells = <2>;
ranges = <0x0 0x04e0 0x00 0x04e0 0x0 0x3>;
  
-		status = "okay";

-
dmas = <_udmap 0xc000>, <_udmap 0x4000>,
<_udmap 0x4001>;
dma-names = "tx", "rx1", "rx2";



--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH 06/25] soc: ti: knav_qmss_queue: Remove set but unchecked variable 'ret'

2020-11-12 Thread Tero Kristo

On 12/11/2020 15:21, Lee Jones wrote:

On Thu, 12 Nov 2020, Tero Kristo wrote:


On 12/11/2020 12:31, Lee Jones wrote:

Cc:ing a few people I know.

On Tue, 03 Nov 2020, Lee Jones wrote:


Fixes the following W=1 kernel build warning(s):

   drivers/soc/ti/knav_qmss_queue.c: In function ‘knav_setup_queue_pools’:
   drivers/soc/ti/knav_qmss_queue.c:1310:6: warning: variable ‘ret’ set but not 
used [-Wunused-but-set-variable]

Cc: Santosh Shilimkar 
Cc: Sandeep Nair 
Cc: Cyril Chemparathy 
Signed-off-by: Lee Jones 
---
   drivers/soc/ti/knav_qmss_queue.c | 3 +--
   1 file changed, 1 insertion(+), 2 deletions(-)


Any idea who will take these TI patches?

https://lore.kernel.org/linux-arm-kernel/2020052540.gh173...@builder.lan/



(Dropped a few inactive emails from delivery.)

Santosh is the maintainer for the subsystem, so my vote would go for him.


Thanks for your prompt reply Tero.

It looks as though Santosh has been on Cc since the start.  He must
just be busy.  I'll give him a little while longer before submitting a
[RESEND].


Yeah, in my experience it can take a while for Santosh to react on 
patches. :)


-Tero


--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH 06/25] soc: ti: knav_qmss_queue: Remove set but unchecked variable 'ret'

2020-11-12 Thread Tero Kristo

On 12/11/2020 12:31, Lee Jones wrote:

Cc:ing a few people I know.

On Tue, 03 Nov 2020, Lee Jones wrote:


Fixes the following W=1 kernel build warning(s):

  drivers/soc/ti/knav_qmss_queue.c: In function ‘knav_setup_queue_pools’:
  drivers/soc/ti/knav_qmss_queue.c:1310:6: warning: variable ‘ret’ set but not 
used [-Wunused-but-set-variable]

Cc: Santosh Shilimkar 
Cc: Sandeep Nair 
Cc: Cyril Chemparathy 
Signed-off-by: Lee Jones 
---
  drivers/soc/ti/knav_qmss_queue.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)


Any idea who will take these TI patches?

https://lore.kernel.org/linux-arm-kernel/2020052540.gh173...@builder.lan/



(Dropped a few inactive emails from delivery.)

Santosh is the maintainer for the subsystem, so my vote would go for him.

-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


[PATCH] opp: fix bad error check logic in the opp helper register

2020-10-29 Thread Tero Kristo
The error check is incorrectly negated causing the helper to never
register anything. This causes platforms that depend on this
functionality to fail always with any cpufreq transition, and at least
TI DRA7 based platforms fail to boot completely due to warning message
flood from _generic_set_opp_regulator complaining about multiple
regulators not being supported.

Fixes: dd461cd9183f ("opp: Allow dev_pm_opp_get_opp_table() to return 
-EPROBE_DEFER")
Signed-off-by: Tero Kristo 
---
 drivers/opp/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 2483e765318a..4ac4e7ce6b8b 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -1930,7 +1930,7 @@ struct opp_table 
*dev_pm_opp_register_set_opp_helper(struct device *dev,
return ERR_PTR(-EINVAL);
 
opp_table = dev_pm_opp_get_opp_table(dev);
-   if (!IS_ERR(opp_table))
+   if (IS_ERR(opp_table))
return opp_table;
 
/* This should be called before OPPs are initialized */
-- 
2.17.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH] opp: fix bad error check logic in the opp helper register

2020-10-29 Thread Tero Kristo

On 28/10/2020 16:54, Viresh Kumar wrote:

On 28-10-20, 16:13, Tero Kristo wrote:

The error check is incorrectly negated causing the helper to never
register anything. This causes platforms that depend on this
functionality to fail always with any cpufreq transition, and at least
TI DRA7 based platforms fail to boot completely due to warning message
flood from _generic_set_opp_regulator complaining about multiple
regulators not being supported.

Fixes: dd461cd9183f ("opp: Allow dev_pm_opp_get_opp_table() to return 
-EPROBE_DEFER")
Signed-off-by: Tero Kristo 
---
  drivers/opp/core.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 2483e765318a..4ac4e7ce6b8b 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -1930,7 +1930,7 @@ struct opp_table 
*dev_pm_opp_register_set_opp_helper(struct device *dev,
return ERR_PTR(-EINVAL);
  
  	opp_table = dev_pm_opp_get_opp_table(dev);

-   if (!IS_ERR(opp_table))
+   if (IS_ERR(opp_table))
return opp_table;
  
  	/* This should be called before OPPs are initialized */


A similar fix is already pushed in linux-next for this.


Ah ok, good to hear. Just checked linux-next and I see the fix also, 
sorry for the noise.


-Tero

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


[RESEND PATCH] soc: ti: ti_sci_pm_domains: check for proper args count in xlate

2020-10-29 Thread Tero Kristo
K2G devices still only use single parameter for power-domains property,
so check for this properly in the driver. Without this, every peripheral
fails to probe resulting in boot failure.

Fixes: efa5c01cd7ee ("soc: ti: ti_sci_pm_domains: switch to use multiple genpds 
instead of one")
Reported-by: Nishanth Menon 
Signed-off-by: Tero Kristo 
Acked-by: Nishanth Menon 
Acked-by: Santosh Shilimkar 
---
 drivers/soc/ti/ti_sci_pm_domains.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/ti/ti_sci_pm_domains.c 
b/drivers/soc/ti/ti_sci_pm_domains.c
index af2126d2b2ff..8afb3f45d263 100644
--- a/drivers/soc/ti/ti_sci_pm_domains.c
+++ b/drivers/soc/ti/ti_sci_pm_domains.c
@@ -91,7 +91,7 @@ static struct generic_pm_domain *ti_sci_pd_xlate(
struct genpd_onecell_data *genpd_data = data;
unsigned int idx = genpdspec->args[0];
 
-   if (genpdspec->args_count < 2)
+   if (genpdspec->args_count != 1 && genpdspec->args_count != 2)
return ERR_PTR(-EINVAL);
 
if (idx >= genpd_data->num_domains) {
-- 
2.17.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


[PATCH] soc: ti: ti_sci_pm_domains: check for proper args count in xlate

2020-10-28 Thread Tero Kristo
K2G devices still only use single parameter for power-domains property,
so check for this properly in the driver. Without this, every peripheral
fails to probe resulting in boot failure.

Fixes: efa5c01cd7ee ("soc: ti: ti_sci_pm_domains: switch to use multiple genpds 
instead of one")
Reported-by: Nishanth Menon 
Signed-off-by: Tero Kristo 
---
 drivers/soc/ti/ti_sci_pm_domains.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/ti/ti_sci_pm_domains.c 
b/drivers/soc/ti/ti_sci_pm_domains.c
index af2126d2b2ff..8afb3f45d263 100644
--- a/drivers/soc/ti/ti_sci_pm_domains.c
+++ b/drivers/soc/ti/ti_sci_pm_domains.c
@@ -91,7 +91,7 @@ static struct generic_pm_domain *ti_sci_pd_xlate(
struct genpd_onecell_data *genpd_data = data;
unsigned int idx = genpdspec->args[0];
 
-   if (genpdspec->args_count < 2)
+   if (genpdspec->args_count != 1 && genpdspec->args_count != 2)
return ERR_PTR(-EINVAL);
 
if (idx >= genpd_data->num_domains) {
-- 
2.17.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


[PATCH] ARM: dts: keystone-k2g: Add stdout-path property

2020-10-28 Thread Tero Kristo
Add stdout-path property in /chosen node so that earlycon can be
used by just adding earlycon in bootargs.

Signed-off-by: Tero Kristo 
---
 arch/arm/boot/dts/keystone-k2g.dtsi | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/keystone-k2g.dtsi 
b/arch/arm/boot/dts/keystone-k2g.dtsi
index 05a75019275e..1105b5d1f886 100644
--- a/arch/arm/boot/dts/keystone-k2g.dtsi
+++ b/arch/arm/boot/dts/keystone-k2g.dtsi
@@ -16,7 +16,9 @@
#size-cells = <2>;
interrupt-parent = <>;
 
-   chosen { };
+   chosen {
+   stdout-path = 
+   };
 
aliases {
serial0 = 
-- 
2.17.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH 0/2] Enable GPIO and I2C configs for TI's J721e platform

2020-10-08 Thread Tero Kristo

On 08/10/2020 12:40, Faiz Abbas wrote:

Tero,

On 08/10/20 2:49 pm, Tero Kristo wrote:

On 08/10/2020 11:59, Faiz Abbas wrote:

Tero,

On 06/10/20 6:40 pm, Tero Kristo wrote:

On 06/10/2020 16:03, Faiz Abbas wrote:

Hi Tero,

On 06/10/20 5:21 pm, Tero Kristo wrote:

On 02/10/2020 19:45, Faiz Abbas wrote:

The following patches enable configs in the arm64 defconfig to support
GPIO and I2C support on TI's J721e platform.

Faiz Abbas (2):
  arm64: defconfig: Enable OMAP I2C driver
  arm64: defconfig: Enable DAVINCI_GPIO driver

     arch/arm64/configs/defconfig | 2 ++
     1 file changed, 2 insertions(+)



Why are you enabling these?

Are they required for booting the board?

If not, they shall not be enabled, as it just clutters the arm64 defconfig 
unnecessarily.



They are required because the SD card regulators need gpio over i2c expander 
and also
soc gpio support to come up in UHS modes.


Is that needed for boot support? If it is only needed with UHS cards, that does 
not seem important enough for me. We can already boot the board via other means.


Without these configs, the regulator drivers keep EPROBE_DEFERing waiting for 
their gpio drivers
to probe and SD card never comes up. This configuration happens before any UHS 
capabilities are detected.

[    1.326654] sdhci-am654 4fb.sdhci: _devm_regulator_get id:vmmc ret:-517
[    1.333651] sdhci-am654 4fb.sdhci: _devm_regulator_get id:vqmmc ret:-517
[    1.340693] sdhci-am654 4fb.sdhci: sdhci_am654_probe ret:-517
[    1.489088] sdhci-am654 4fb.sdhci: _devm_regulator_get id:vmmc ret:-517
[    1.496067] sdhci-am654 4fb.sdhci: _devm_regulator_get id:vqmmc ret:-517
[    1.510392] sdhci-am654 4fb.sdhci: sdhci_am654_probe ret:-517
[    1.543210] sdhci-am654 4fb.sdhci: _devm_regulator_get id:vmmc ret:-517
[    1.550186] sdhci-am654 4fb.sdhci: _devm_regulator_get id:vqmmc ret:-517
[    1.568134] sdhci-am654 4fb.sdhci: sdhci_am654_probe ret:-517


This happens because you have merged/enabled UHS support or? This sounds like a 
regression as I haven't seen this happen before.



Thats right. The EPROBE_DEFERs will happen if my patches enabling UHS modes 
here are merged. I need to repost them for v5.11-rc1:
https://lore.kernel.org/linux-arm-kernel/20201001190541.6364-1-faiz_ab...@ti.com/


Ok I think that would be good enough reason to enable these by default 
as the MMC as boot media won't work anymore without them, and carrying 
the DTS patches would be just silly.


Acked-by: Tero Kristo 

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH 0/2] Enable GPIO and I2C configs for TI's J721e platform

2020-10-08 Thread Tero Kristo

On 08/10/2020 11:59, Faiz Abbas wrote:

Tero,

On 06/10/20 6:40 pm, Tero Kristo wrote:

On 06/10/2020 16:03, Faiz Abbas wrote:

Hi Tero,

On 06/10/20 5:21 pm, Tero Kristo wrote:

On 02/10/2020 19:45, Faiz Abbas wrote:

The following patches enable configs in the arm64 defconfig to support
GPIO and I2C support on TI's J721e platform.

Faiz Abbas (2):
     arm64: defconfig: Enable OMAP I2C driver
     arm64: defconfig: Enable DAVINCI_GPIO driver

    arch/arm64/configs/defconfig | 2 ++
    1 file changed, 2 insertions(+)



Why are you enabling these?

Are they required for booting the board?

If not, they shall not be enabled, as it just clutters the arm64 defconfig 
unnecessarily.



They are required because the SD card regulators need gpio over i2c expander 
and also
soc gpio support to come up in UHS modes.


Is that needed for boot support? If it is only needed with UHS cards, that does 
not seem important enough for me. We can already boot the board via other means.


Without these configs, the regulator drivers keep EPROBE_DEFERing waiting for 
their gpio drivers
to probe and SD card never comes up. This configuration happens before any UHS 
capabilities are detected.

[1.326654] sdhci-am654 4fb.sdhci: _devm_regulator_get id:vmmc ret:-517
[1.333651] sdhci-am654 4fb.sdhci: _devm_regulator_get id:vqmmc ret:-517
[1.340693] sdhci-am654 4fb.sdhci: sdhci_am654_probe ret:-517
[1.489088] sdhci-am654 4fb.sdhci: _devm_regulator_get id:vmmc ret:-517
[1.496067] sdhci-am654 4fb.sdhci: _devm_regulator_get id:vqmmc ret:-517
[1.510392] sdhci-am654 4fb.sdhci: sdhci_am654_probe ret:-517
[1.543210] sdhci-am654 4fb.sdhci: _devm_regulator_get id:vmmc ret:-517
[1.550186] sdhci-am654 4fb.sdhci: _devm_regulator_get id:vqmmc ret:-517
[1.568134] sdhci-am654 4fb.sdhci: sdhci_am654_probe ret:-517


This happens because you have merged/enabled UHS support or? This sounds 
like a regression as I haven't seen this happen before.


-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH 0/2] Enable GPIO and I2C configs for TI's J721e platform

2020-10-06 Thread Tero Kristo

On 06/10/2020 16:03, Faiz Abbas wrote:

Hi Tero,

On 06/10/20 5:21 pm, Tero Kristo wrote:

On 02/10/2020 19:45, Faiz Abbas wrote:

The following patches enable configs in the arm64 defconfig to support
GPIO and I2C support on TI's J721e platform.

Faiz Abbas (2):
    arm64: defconfig: Enable OMAP I2C driver
    arm64: defconfig: Enable DAVINCI_GPIO driver

   arch/arm64/configs/defconfig | 2 ++
   1 file changed, 2 insertions(+)



Why are you enabling these?

Are they required for booting the board?

If not, they shall not be enabled, as it just clutters the arm64 defconfig 
unnecessarily.



They are required because the SD card regulators need gpio over i2c expander 
and also
soc gpio support to come up in UHS modes.


Is that needed for boot support? If it is only needed with UHS cards, 
that does not seem important enough for me. We can already boot the 
board via other means.




But in general isn't any feature we add supposed to be enabled in the arm64 
defconfig?


That is debatable, as it just increases the kernel size / build time for 
everybody. Personally I would merge only things that are absolutely 
necessary, for everything else we can just do local config modifications.


-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH 0/2] Enable GPIO and I2C configs for TI's J721e platform

2020-10-06 Thread Tero Kristo

On 02/10/2020 19:45, Faiz Abbas wrote:

The following patches enable configs in the arm64 defconfig to support
GPIO and I2C support on TI's J721e platform.

Faiz Abbas (2):
   arm64: defconfig: Enable OMAP I2C driver
   arm64: defconfig: Enable DAVINCI_GPIO driver

  arch/arm64/configs/defconfig | 2 ++
  1 file changed, 2 insertions(+)



Why are you enabling these?

Are they required for booting the board?

If not, they shall not be enabled, as it just clutters the arm64 
defconfig unnecessarily.


-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


[PATCH] soc: ti: ti_sci_pm_domains: switch to use multiple genpds instead of one

2020-09-07 Thread Tero Kristo
Current implementation of the genpd support over TI SCI uses a single
genpd across the whole SoC, and attaches multiple devices to this. This
solution has its drawbacks, like it is currently impossible to attach
more than one power domain to a device; the core genpd implementation
requires one genpd per power-domain entry in DT for a single device.
Also, some devices like USB apparently require their own genpd during
probe time, the current shared approach in use does not work at all.

Switch the implementation over to use a single genpd per power domain
entry in DT. The domains are registered with the onecell approach, but
we also add our own xlate service due to recent introduction of the
extended flag for TI SCI PM domains; genpd core xlate service requires
a single cell per powerdomain, but we are using two cells.

Signed-off-by: Tero Kristo 
---
 drivers/soc/ti/ti_sci_pm_domains.c | 251 ++---
 1 file changed, 121 insertions(+), 130 deletions(-)

diff --git a/drivers/soc/ti/ti_sci_pm_domains.c 
b/drivers/soc/ti/ti_sci_pm_domains.c
index 8c2a2f23982c..af2126d2b2ff 100644
--- a/drivers/soc/ti/ti_sci_pm_domains.c
+++ b/drivers/soc/ti/ti_sci_pm_domains.c
@@ -9,7 +9,6 @@
 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -18,150 +17,95 @@
 #include 
 
 /**
- * struct ti_sci_genpd_dev_data: holds data needed for every device attached
- *  to this genpd
- * @idx: index of the device that identifies it with the system
- *  control processor.
- * @exclusive: Permissions for exclusive request or shared request of the
- *device.
+ * struct ti_sci_genpd_provider: holds common TI SCI genpd provider data
+ * @ti_sci: handle to TI SCI protocol driver that provides ops to
+ * communicate with system control processor.
+ * @dev: pointer to dev for the driver for devm allocs
+ * @pd_list: list of all the power domains on the device
+ * @data: onecell data for genpd core
  */
-struct ti_sci_genpd_dev_data {
-   int idx;
-   u8 exclusive;
+struct ti_sci_genpd_provider {
+   const struct ti_sci_handle *ti_sci;
+   struct device *dev;
+   struct list_head pd_list;
+   struct genpd_onecell_data data;
 };
 
 /**
  * struct ti_sci_pm_domain: TI specific data needed for power domain
- * @ti_sci: handle to TI SCI protocol driver that provides ops to
- * communicate with system control processor.
- * @dev: pointer to dev for the driver for devm allocs
+ * @idx: index of the device that identifies it with the system
+ *  control processor.
+ * @exclusive: Permissions for exclusive request or shared request of the
+ *device.
  * @pd: generic_pm_domain for use with the genpd framework
+ * @node: link for the genpd list
+ * @parent: link to the parent TI SCI genpd provider
  */
 struct ti_sci_pm_domain {
-   const struct ti_sci_handle *ti_sci;
-   struct device *dev;
+   int idx;
+   u8 exclusive;
struct generic_pm_domain pd;
+   struct list_head node;
+   struct ti_sci_genpd_provider *parent;
 };
 
 #define genpd_to_ti_sci_pd(gpd) container_of(gpd, struct ti_sci_pm_domain, pd)
 
-/**
- * ti_sci_dev_id(): get prepopulated ti_sci id from struct dev
- * @dev: pointer to device associated with this genpd
- *
- * Returns device_id stored from ti,sci_id property
- */
-static int ti_sci_dev_id(struct device *dev)
-{
-   struct generic_pm_domain_data *genpd_data = dev_gpd_data(dev);
-   struct ti_sci_genpd_dev_data *sci_dev_data = genpd_data->data;
-
-   return sci_dev_data->idx;
-}
-
-static u8 is_ti_sci_dev_exclusive(struct device *dev)
-{
-   struct generic_pm_domain_data *genpd_data = dev_gpd_data(dev);
-   struct ti_sci_genpd_dev_data *sci_dev_data = genpd_data->data;
-
-   return sci_dev_data->exclusive;
-}
-
-/**
- * ti_sci_dev_to_sci_handle(): get pointer to ti_sci_handle
- * @dev: pointer to device associated with this genpd
- *
- * Returns ti_sci_handle to be used to communicate with system
- *control processor.
+/*
+ * ti_sci_pd_power_off(): genpd power down hook
+ * @domain: pointer to the powerdomain to power off
  */
-static const struct ti_sci_handle *ti_sci_dev_to_sci_handle(struct device *dev)
+static int ti_sci_pd_power_off(struct generic_pm_domain *domain)
 {
-   struct generic_pm_domain *pd = pd_to_genpd(dev->pm_domain);
-   struct ti_sci_pm_domain *ti_sci_genpd = genpd_to_ti_sci_pd(pd);
+   struct ti_sci_pm_domain *pd = genpd_to_ti_sci_pd(domain);
+   const struct ti_sci_handle *ti_sci = pd->parent->ti_sci;
 
-   return ti_sci_genpd->ti_sci;
+   return ti_sci->ops.dev_ops.put_device(ti_sci, pd->idx);
 }
 
-/**
- * ti_sci_dev_start(): genpd device start hook called to turn device on
- * @dev: pointer to device associated with this genpd to be powered on
+/*
+ * ti_sci_pd_power_on(): genpd power up hook
+ * @domain: pointer to the powerdomain to power on
  *

[PATCH] firmware: ti_sci: allow frequency change for disabled clocks by default

2020-09-07 Thread Tero Kristo
If a clock is disabled, its frequency should be allowed to change as
it is no longer in use. Add a flag towards this to the firmware clock
API handler routines.

Tested-by: Tomi Valkeinen 
Signed-off-by: Tero Kristo 
---
 drivers/firmware/ti_sci.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 53cee17d0115..39890665a975 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -1124,7 +1124,8 @@ static int ti_sci_cmd_get_clock(const struct 
ti_sci_handle *handle, u32 dev_id,
 static int ti_sci_cmd_idle_clock(const struct ti_sci_handle *handle,
 u32 dev_id, u32 clk_id)
 {
-   return ti_sci_set_clock_state(handle, dev_id, clk_id, 0,
+   return ti_sci_set_clock_state(handle, dev_id, clk_id,
+ MSG_FLAG_CLOCK_ALLOW_FREQ_CHANGE,
  MSG_CLOCK_SW_STATE_UNREQ);
 }
 
@@ -1143,7 +1144,8 @@ static int ti_sci_cmd_idle_clock(const struct 
ti_sci_handle *handle,
 static int ti_sci_cmd_put_clock(const struct ti_sci_handle *handle,
u32 dev_id, u32 clk_id)
 {
-   return ti_sci_set_clock_state(handle, dev_id, clk_id, 0,
+   return ti_sci_set_clock_state(handle, dev_id, clk_id,
+ MSG_FLAG_CLOCK_ALLOW_FREQ_CHANGE,
  MSG_CLOCK_SW_STATE_AUTO);
 }
 
-- 
2.17.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


[PATCH 3/3] clk: keystone: sci-clk: add 10% slack to set_rate

2020-09-07 Thread Tero Kristo
Currently, we request exact clock rates from the firmware to be set with
set_rate. Due to some rounding errors and internal functionality of the
firmware itself, this can fail. Thus, add some slack to the set_rate
functionality so that we are always guaranteed to pass. The firmware
always attempts to use frequency as close to the target freq as
possible despite the slack given here.

Signed-off-by: Tero Kristo 
---
 drivers/clk/keystone/sci-clk.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/keystone/sci-clk.c b/drivers/clk/keystone/sci-clk.c
index b6477b08af04..aaf31abe1c8f 100644
--- a/drivers/clk/keystone/sci-clk.c
+++ b/drivers/clk/keystone/sci-clk.c
@@ -221,7 +221,8 @@ static int sci_clk_set_rate(struct clk_hw *hw, unsigned 
long rate,
struct sci_clk *clk = to_sci_clk(hw);
 
return clk->provider->ops->set_freq(clk->provider->sci, clk->dev_id,
-   clk->clk_id, rate, rate, rate);
+   clk->clk_id, rate / 10 * 9, rate,
+   rate / 10 * 11);
 }
 
 /**
-- 
2.17.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


[PATCH 1/3] clk: keystone: sci-clk: fix parsing assigned-clock data during probe

2020-09-07 Thread Tero Kristo
The DT clock probe loop incorrectly terminates after processing "clocks"
only, fix this by re-starting the loop when all entries for current
DT property have been parsed.

Fixes: 8e48b33f9def ("clk: keystone: sci-clk: probe clocks from DT instead of 
firmware")
Reported-by: Peter Ujfalusi 
Signed-off-by: Tero Kristo 
---
 drivers/clk/keystone/sci-clk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/keystone/sci-clk.c b/drivers/clk/keystone/sci-clk.c
index 2ad26cb927fd..f126b6045afa 100644
--- a/drivers/clk/keystone/sci-clk.c
+++ b/drivers/clk/keystone/sci-clk.c
@@ -522,7 +522,7 @@ static int ti_sci_scan_clocks_from_dt(struct 
sci_clk_provider *provider)
np = of_find_node_with_property(np, *clk_name);
if (!np) {
clk_name++;
-   break;
+   continue;
}
 
if (!of_device_is_available(np))
-- 
2.17.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


[PATCH 0/3] clk: keystone: some minor fixes

2020-09-07 Thread Tero Kristo
Hi Santosh,

This series contains a few fixes for the TI SCI clock driver.
- Patch #1 is a clear bug fix, where we missed to parse assigned-clock
  data properly to detect which clocks are in use on the SoC.
- Patch #2 is a performance improvement patch which avoids some
  unnecessary round trips to firmware side while setting clock
  frequency.
- Patch #3 fixes some issues with set_rate passed to firmware, where the
  parameters are too strict; namely, firmware fails to handle some cases
  properly if min,tgt,max values for a clock rate are exactly the same
  value. Yeah, the firmware is quite weird here but nothing much else we
  can do from kernel side other than this

-Tero


--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


[PATCH 2/3] clk: keystone: sci-clk: cache results of last query rate operation

2020-09-07 Thread Tero Kristo
Cache results of the latest query rate operation. This optimizes the
firmware interface a bit, avoiding unnecessary calls to firmware if we
know the result already; the firmware interface is pretty expensive
to use for query rate functionality.

Signed-off-by: Tero Kristo 
---
 drivers/clk/keystone/sci-clk.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/drivers/clk/keystone/sci-clk.c b/drivers/clk/keystone/sci-clk.c
index f126b6045afa..b6477b08af04 100644
--- a/drivers/clk/keystone/sci-clk.c
+++ b/drivers/clk/keystone/sci-clk.c
@@ -54,6 +54,8 @@ struct sci_clk_provider {
  * @provider:   Master clock provider
  * @flags:  Flags for the clock
  * @node:   Link for handling clocks probed via DT
+ * @cached_req: Cached requested freq for determine rate calls
+ * @cached_res: Cached result freq for determine rate calls
  */
 struct sci_clk {
struct clk_hw hw;
@@ -63,6 +65,8 @@ struct sci_clk {
struct sci_clk_provider *provider;
u8 flags;
struct list_head node;
+   unsigned long cached_req;
+   unsigned long cached_res;
 };
 
 #define to_sci_clk(_hw) container_of(_hw, struct sci_clk, hw)
@@ -175,6 +179,11 @@ static int sci_clk_determine_rate(struct clk_hw *hw,
int ret;
u64 new_rate;
 
+   if (clk->cached_req && clk->cached_req == req->rate) {
+   req->rate = clk->cached_res;
+   return 0;
+   }
+
ret = clk->provider->ops->get_best_match_freq(clk->provider->sci,
  clk->dev_id,
  clk->clk_id,
@@ -189,6 +198,9 @@ static int sci_clk_determine_rate(struct clk_hw *hw,
return ret;
}
 
+   clk->cached_req = req->rate;
+   clk->cached_res = new_rate;
+
req->rate = new_rate;
 
return 0;
@@ -249,6 +261,8 @@ static int sci_clk_set_parent(struct clk_hw *hw, u8 index)
 {
struct sci_clk *clk = to_sci_clk(hw);
 
+   clk->cached_req = 0;
+
return clk->provider->ops->set_parent(clk->provider->sci, clk->dev_id,
  clk->clk_id,
  index + 1 + clk->clk_id);
-- 
2.17.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH 2/2] EDAC/ti: Fix handling of platform_get_irq() error

2020-08-27 Thread Tero Kristo

On 27/08/2020 10:07, Krzysztof Kozlowski wrote:

platform_get_irq() returns -ERRNO on error.  In such case comparison
to 0 would pass the check.

Fixes: 86a18ee21e5e ("EDAC, ti: Add support for TI keystone and DRA7xx EDAC")
Signed-off-by: Krzysztof Kozlowski 


Reviewed-by: Tero Kristo 


---
  drivers/edac/ti_edac.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/edac/ti_edac.c b/drivers/edac/ti_edac.c
index 6e52796a0b41..e7eae20f83d1 100644
--- a/drivers/edac/ti_edac.c
+++ b/drivers/edac/ti_edac.c
@@ -278,7 +278,8 @@ static int ti_edac_probe(struct platform_device *pdev)
  
  	/* add EMIF ECC error handler */

error_irq = platform_get_irq(pdev, 0);
-   if (!error_irq) {
+   if (error_irq < 0) {
+   ret = error_irq;
edac_printk(KERN_ERR, EDAC_MOD_NAME,
"EMIF irq number not defined.\n");
goto err;



--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH] crypto: sa2ul - Reduce stack usage

2020-08-25 Thread Tero Kristo

On 24/08/2020 16:12, Herbert Xu wrote:

On Sun, Aug 16, 2020 at 02:56:43PM +0800, kernel test robot wrote:


drivers/crypto/sa2ul.c: In function 'sa_prepare_iopads':

drivers/crypto/sa2ul.c:432:1: warning: the frame size of 1076 bytes is larger 
than 1024 bytes [-Wframe-larger-than=]


This patch tries to reduce the stack frame size in sa2ul.

---8<---
This patch reduces the stack usage in sa2ul:

1. Move the exported sha state into sa_prepare_iopads so that it
can occupy the same space as the k_pad buffer.

2. Use one buffer for ipad/opad in sa_prepare_iopads.

3. Remove ipad/opad buffer from sa_set_sc_auth.

4. Use async skcipher fallback and remove on-stack request from
sa_cipher_run.

Reported-by: kernel test robot 
Fixes: d2c8ac187fc9 ("crypto: sa2ul - Add AEAD algorithm support")
Signed-off-by: Herbert Xu 


This patch seems ok, I also tested it locally so:

Reviewed-by: Tero Kristo 
Tested-by: Tero Kristo 



diff --git a/drivers/crypto/sa2ul.c b/drivers/crypto/sa2ul.c
index 5bc099052bd2..66629cad9531 100644
--- a/drivers/crypto/sa2ul.c
+++ b/drivers/crypto/sa2ul.c
@@ -9,8 +9,10 @@
   *        Tero Kristo
   */
  #include 
+#include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -356,42 +358,45 @@ static void sa_swiz_128(u8 *in, u16 len)
  }
  
  /* Prepare the ipad and opad from key as per SHA algorithm step 1*/

-static void prepare_kiopad(u8 *k_ipad, u8 *k_opad, const u8 *key, u16 key_sz)
+static void prepare_kipad(u8 *k_ipad, const u8 *key, u16 key_sz)
  {
int i;
  
-	for (i = 0; i < key_sz; i++) {

+   for (i = 0; i < key_sz; i++)
k_ipad[i] = key[i] ^ 0x36;
-   k_opad[i] = key[i] ^ 0x5c;
-   }
  
  	/* Instead of XOR with 0 */

-   for (; i < SHA1_BLOCK_SIZE; i++) {
+   for (; i < SHA1_BLOCK_SIZE; i++)
k_ipad[i] = 0x36;
+}
+
+static void prepare_kopad(u8 *k_opad, const u8 *key, u16 key_sz)
+{
+   int i;
+
+   for (i = 0; i < key_sz; i++)
+   k_opad[i] = key[i] ^ 0x5c;
+
+   /* Instead of XOR with 0 */
+   for (; i < SHA1_BLOCK_SIZE; i++)
k_opad[i] = 0x5c;
-   }
  }
  
-static void sa_export_shash(struct shash_desc *hash, int block_size,

+static void sa_export_shash(void *state, struct shash_desc *hash,
int digest_size, __be32 *out)
  {
-   union {
-   struct sha1_state sha1;
-   struct sha256_state sha256;
-   struct sha512_state sha512;
-   } sha;
-   void *state;
+   struct sha1_state *sha1;
+   struct sha256_state *sha256;
u32 *result;
-   int i;
  
  	switch (digest_size) {

case SHA1_DIGEST_SIZE:
-   state = 
-   result = sha.sha1.state;
+   sha1 = state;
+   result = sha1->state;
break;
case SHA256_DIGEST_SIZE:
-   state = 
-   result = sha.sha256.state;
+   sha256 = state;
+   result = sha256->state;
break;
default:
dev_err(sa_k3_dev, "%s: bad digest_size=%d\n", __func__,
@@ -401,8 +406,7 @@ static void sa_export_shash(struct shash_desc *hash, int 
block_size,
  
  	crypto_shash_export(hash, state);
  
-	for (i = 0; i < digest_size >> 2; i++)

-   out[i] = cpu_to_be32(result[i]);
+   cpu_to_be32_array(out, result, digest_size / 4);
  }
  
  static void sa_prepare_iopads(struct algo_data *data, const u8 *key,

@@ -411,24 +415,28 @@ static void sa_prepare_iopads(struct algo_data *data, 
const u8 *key,
SHASH_DESC_ON_STACK(shash, data->ctx->shash);
int block_size = crypto_shash_blocksize(data->ctx->shash);
int digest_size = crypto_shash_digestsize(data->ctx->shash);
-   u8 k_ipad[SHA1_BLOCK_SIZE];
-   u8 k_opad[SHA1_BLOCK_SIZE];
+   union {
+   struct sha1_state sha1;
+   struct sha256_state sha256;
+   u8 k_pad[SHA1_BLOCK_SIZE];
+   } sha;
  
  	shash->tfm = data->ctx->shash;
  
-	prepare_kiopad(k_ipad, k_opad, key, key_sz);

-
-   memzero_explicit(ipad, block_size);
-   memzero_explicit(opad, block_size);
+   prepare_kipad(sha.k_pad, key, key_sz);
  
  	crypto_shash_init(shash);

-   crypto_shash_update(shash, k_ipad, block_size);
-   sa_export_shash(shash, block_size, digest_size, ipad);
+   crypto_shash_update(shash, sha.k_pad, block_size);
+   sa_export_shash(, shash, digest_size, ipad);
+
+   prepare_kopad(sha.k_pad, key, key_sz);
  
  	crypto_shash_init(shash);

-   crypto_shash_update(shash, k_opad, block_size);
+   crypto_shash_update(shash, sha.k_pad, block_size);
  
-	sa_export_shash(shash, block_size, digest_size, opad);

+   sa_export_shash(, shash, digest_size, opad);
+
+   memzero_explicit(, sizeof(sha));
  }
  
  /* Derive the inverse key us

Re: [PATCH] firmware: ti_sci: Replace HTTP links with HTTPS ones

2020-07-20 Thread Tero Kristo

Hi Alexander,

One comment below.

On 18/07/2020 13:55, Alexander A. Klimov wrote:

Rationale:
Reduces attack surface on kernel devs opening the links for MITM
as HTTPS traffic is much harder to manipulate.

Deterministic algorithm:
For each file:
   If not .svg:
 For each line:
   If doesn't contain `\bxmlns\b`:
 For each link, `\bhttp://[^# \t\r\n]*(?:\w|/)`:
  If neither `\bgnu\.org/license`, nor `\bmozilla\.org/MPL\b`:
 If both the HTTP and HTTPS versions
 return 200 OK and serve the same content:
   Replace HTTP with HTTPS.

Signed-off-by: Alexander A. Klimov 
---
  Continuing my work started at 93431e0607e5.
  See also: git log --oneline '--author=Alexander A. Klimov 
' v5.7..master

  If there are any URLs to be removed completely
  or at least not (just) HTTPSified:
  Just clearly say so and I'll *undo my change*.
  See also: https://lkml.org/lkml/2020/6/27/64

  If there are any valid, but yet not changed URLs:
  See: https://lkml.org/lkml/2020/6/26/837

  If you apply the patch, please let me know.


  .../devicetree/bindings/interrupt-controller/ti,sci-intr.txt| 2 +-
  drivers/firmware/ti_sci.c   | 2 +-
  drivers/firmware/ti_sci.h   | 2 +-
  drivers/irqchip/irq-ti-sci-inta.c   | 2 +-
  drivers/irqchip/irq-ti-sci-intr.c   | 2 +-
  drivers/reset/reset-ti-sci.c| 2 +-
  include/linux/soc/ti/ti_sci_inta_msi.h  | 2 +-
  include/linux/soc/ti/ti_sci_protocol.h  | 2 +-
  8 files changed, 8 insertions(+), 8 deletions(-)

diff --git 
a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt 
b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
index 1a8718f8855d..178fca08278f 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
@@ -55,7 +55,7 @@ Required Properties:
corresponds to a range of host irqs.
  
  For more details on TISCI IRQ resource management refer:

-http://downloads.ti.com/tisci/esd/latest/2_tisci_msgs/rm/rm_irq.html
+https://downloads.ti.com/tisci/esd/latest/2_tisci_msgs/rm/rm_irq.html
  
  Example:

  
diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 4126be9e3216..53cee17d0115 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -2,7 +2,7 @@
  /*
   * Texas Instruments System Control Interface Protocol Driver
   *
- * Copyright (C) 2015-2016 Texas Instruments Incorporated - http://www.ti.com/
+ * Copyright (C) 2015-2016 Texas Instruments Incorporated - https://www.ti.com/
   *Nishanth Menon
   */
  
diff --git a/drivers/firmware/ti_sci.h b/drivers/firmware/ti_sci.h

index f0d068c03944..57cd04062994 100644
--- a/drivers/firmware/ti_sci.h
+++ b/drivers/firmware/ti_sci.h
@@ -6,7 +6,7 @@
   * The system works in a message response protocol
   * See: http://processors.wiki.ti.com/index.php/TISCI for details




You should probably replace that one as well to be https while doing the 
rest of the changes, even though the wiki is being deprecated during 
this year.


-Tero


   *
- * Copyright (C)  2015-2016 Texas Instruments Incorporated - http://www.ti.com/
+ * Copyright (C)  2015-2016 Texas Instruments Incorporated - 
https://www.ti.com/
   */
  
  #ifndef __TI_SCI_H

diff --git a/drivers/irqchip/irq-ti-sci-inta.c 
b/drivers/irqchip/irq-ti-sci-inta.c
index 7e3ebf6ed2cd..85de19fe9b6e 100644
--- a/drivers/irqchip/irq-ti-sci-inta.c
+++ b/drivers/irqchip/irq-ti-sci-inta.c
@@ -2,7 +2,7 @@
  /*
   * Texas Instruments' K3 Interrupt Aggregator irqchip driver
   *
- * Copyright (C) 2018-2019 Texas Instruments Incorporated - http://www.ti.com/
+ * Copyright (C) 2018-2019 Texas Instruments Incorporated - https://www.ti.com/
   *Lokesh Vutla 
   */
  
diff --git a/drivers/irqchip/irq-ti-sci-intr.c b/drivers/irqchip/irq-ti-sci-intr.c

index 59d51a20bbd8..5ea148faf2ab 100644
--- a/drivers/irqchip/irq-ti-sci-intr.c
+++ b/drivers/irqchip/irq-ti-sci-intr.c
@@ -2,7 +2,7 @@
  /*
   * Texas Instruments' K3 Interrupt Router irqchip driver
   *
- * Copyright (C) 2018-2019 Texas Instruments Incorporated - http://www.ti.com/
+ * Copyright (C) 2018-2019 Texas Instruments Incorporated - https://www.ti.com/
   *Lokesh Vutla 
   */
  
diff --git a/drivers/reset/reset-ti-sci.c b/drivers/reset/reset-ti-sci.c

index bf68729ab729..b799aefad547 100644
--- a/drivers/reset/reset-ti-sci.c
+++ b/drivers/reset/reset-ti-sci.c
@@ -1,7 +1,7 @@
  /*
   * Texas Instrument's System Control Interface (TI-SCI) reset driver
   *
- * Copyright (C) 2015-2017 Texas Instruments Incorporated - http://www.ti.com/
+ * Copyright (C) 2015-2017 Texas Instruments Incorporated - https://www.ti.com/
   *Andrew F. Davis 
   *
   * This program 

Re: [PATCH] soc: ti/ti_sci_protocol.h: drop a duplicated word + clarify

2020-07-20 Thread Tero Kristo

On 19/07/2020 03:31, Randy Dunlap wrote:

Drop the repeated word "an" in a comment.
Insert "and" between "source" and "destination" as is done a few
lines earlier.

Signed-off-by: Randy Dunlap 
Cc: Nishanth Menon 
Cc: Tero Kristo 
Cc: Santosh Shilimkar 
Cc: Lokesh Vutla 
Cc: Arnd Bergmann 
Cc: linux-arm-ker...@lists.infradead.org


Reviewed-by: Tero Kristo 


---
  include/linux/soc/ti/ti_sci_protocol.h |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

--- linux-next-20200717.orig/include/linux/soc/ti/ti_sci_protocol.h
+++ linux-next-20200717/include/linux/soc/ti/ti_sci_protocol.h
@@ -226,8 +226,8 @@ struct ti_sci_rm_core_ops {
   *and destination
   * @set_event_map:Set an Event based peripheral irq to Interrupt
   *Aggregator.
- * @free_irq:  Free an an IRQ route between the requested source
- * destination.
+ * @free_irq:  Free an IRQ route between the requested source
+ * and destination.
   * @free_event_map:   Free an event based peripheral irq to Interrupt
   *Aggregator.
   */



--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH] arm64: arch_k3: enable chipid driver

2020-07-17 Thread Tero Kristo

On 15/07/2020 13:03, Grygorii Strashko wrote:

Hi All,

On 07/07/2020 10:02, Peter Ujfalusi wrote:

On 01/07/2020 13.18, Grygorii Strashko wrote:

On 19/06/2020 19:25, Grygorii Strashko wrote:

Select TI chip id driver for TI's SoCs based on K3 architecture to
provide
this information to user space and Kernel as it is required by other
drivers to determine SoC revision to function properly.

Signed-off-by: Grygorii Strashko 
Reviewed-by: Lokesh Vutla 
---
   arch/arm64/Kconfig.platforms | 1 +
   1 file changed, 1 insertion(+)

diff --git a/arch/arm64/Kconfig.platforms 
b/arch/arm64/Kconfig.platforms

index 8dd05b2a925c..1d3710e3626a 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -98,6 +98,7 @@ config ARCH_K3
   select TI_SCI_PROTOCOL
   select TI_SCI_INTR_IRQCHIP
   select TI_SCI_INTA_IRQCHIP
+    select TI_K3_SOCINFO
   help
 This enables support for Texas Instruments' K3 multicore SoC
 architecture.



Are there any comments?


Reviewed-by: Peter Ujfalusi 
Tested-by: Peter Ujfalusi 

The driver and dt changes were merged [1][2] and for adding new 
am654x SoC

revision SR2.0 we need this driver to be enabled always as other drivers
are
going to use SOC Bus API intensively.

No dependencies.

[1]
https://lore.kernel.org/lkml/20200512123449.16517-1-grygorii.stras...@ti.com/ 



[2]
https://lore.kernel.org/lkml/20200613164346.28852-1-grygorii.stras...@ti.com/ 





Any chances it can be applied for 5.9?


Queuing up for 5.9, thanks.

Sorry there has been some confusion on my behalf regarding the config 
changes.


-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH 7/7] arm64: defconfig: Enable AM654x SDHCI controller

2020-07-17 Thread Tero Kristo

On 17/07/2020 16:09, Arnd Bergmann wrote:

On Fri, Jul 17, 2020 at 1:20 PM Tero Kristo  wrote:

On 17/07/2020 11:38, Faiz Abbas wrote:

On 16/07/20 11:58 pm, Arnd Bergmann wrote:

On Thu, Jul 16, 2020 at 3:25 PM Sekhar Nori  wrote:
I tend to ignore individual patches to the defconfig file unless
they are sent to:s...@kernel.org. The best way to get them
included is to have the platform maintainers pick up the
changes and send them that way as a separate pull request
at the same time as sending any DT updates.

The MAINTAINERS file lists Tero and Nishanth as maintainers
for the platform. If they want, I can apply this one directly, but in
the future, send it to them.



Thanks for clarifying Arnd. Tero, can you pick this up?


Ok, this topic has been bit unclear for me also, but if you say I can
pick the patches myself and send a pull request out, I can do that.


Right. To clarify, the soc tree usually has separate branches for dts
files, soc specific drivers, defconfig files and 32-bit platform code.

When you pick up patches into your tree, please put them into
branches that fit into those categories. You can group the patches
into branches with more fine-grained categories if it makes sense
(e.g. adding a particularly large driver, adding a new dts files for a
new soc, or cosmetic cleanups across dts files).

If any of the categories only have a couple of patches in them, you
can decide to forward those as patches to s...@kernel.org, but a
pull request is always ok as well, even for a one-line patch.


Ok thanks for clarification, Arnd.

Based on that, queuing this up for 5.9 myself, thanks.

Will post pull-request next week for it, there appears to be another K3 
SoC related config change pending which I'll pick up also. Just want to 
capture -next results for these to see how well they integrate.


-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


[PATCHv4 0/4] watchdog: rti-wdt: attach to running timer

2020-07-17 Thread Tero Kristo
Hi,

V4 of this series only fixes 32bit compile issue with patch #3. Appears
to be compiling now fine with ARM32 allmodconfig.

-Tero


--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


[PATCHv4 1/4] watchdog: use __watchdog_ping in startup

2020-07-17 Thread Tero Kristo
Current watchdog startup functionality does not respect the minimum hw
heartbeat setup and the last watchdog ping timeframe when watchdog is
already running and userspace process attaches to it. Fix this by using
the __watchdog_ping from the startup also. For this code path, we can
also let the __watchdog_ping handle the bookkeeping for the worker and
last keepalive times.

Signed-off-by: Tero Kristo 
Reviewed-by: Guenter Roeck 
---
 drivers/watchdog/watchdog_dev.c | 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 7e4cd34a8c20..bc1cfa288553 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -275,15 +275,18 @@ static int watchdog_start(struct watchdog_device *wdd)
set_bit(_WDOG_KEEPALIVE, _data->status);
 
started_at = ktime_get();
-   if (watchdog_hw_running(wdd) && wdd->ops->ping)
-   err = wdd->ops->ping(wdd);
-   else
+   if (watchdog_hw_running(wdd) && wdd->ops->ping) {
+   err = __watchdog_ping(wdd);
+   if (err == 0)
+   set_bit(WDOG_ACTIVE, >status);
+   } else {
err = wdd->ops->start(wdd);
-   if (err == 0) {
-   set_bit(WDOG_ACTIVE, >status);
-   wd_data->last_keepalive = started_at;
-   wd_data->last_hw_keepalive = started_at;
-   watchdog_update_worker(wdd);
+   if (err == 0) {
+   set_bit(WDOG_ACTIVE, >status);
+   wd_data->last_keepalive = started_at;
+   wd_data->last_hw_keepalive = started_at;
+   watchdog_update_worker(wdd);
+   }
}
 
return err;
-- 
2.17.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


[PATCHv4 4/4] watchdog: rti-wdt: balance pm runtime enable calls

2020-07-17 Thread Tero Kristo
PM runtime should be disabled in the fail path of probe and when
the driver is removed.

Fixes: 2d63908bdbfb ("watchdog: Add K3 RTI watchdog support")
Signed-off-by: Tero Kristo 
Reviewed-by: Guenter Roeck 
---
 drivers/watchdog/rti_wdt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
index 7cbdc178ffe8..705e8f7523e8 100644
--- a/drivers/watchdog/rti_wdt.c
+++ b/drivers/watchdog/rti_wdt.c
@@ -303,6 +303,7 @@ static int rti_wdt_probe(struct platform_device *pdev)
 
 err_iomap:
pm_runtime_put_sync(>dev);
+   pm_runtime_disable(>dev);
 
return ret;
 }
@@ -313,6 +314,7 @@ static int rti_wdt_remove(struct platform_device *pdev)
 
watchdog_unregister_device(>wdd);
pm_runtime_put(>dev);
+   pm_runtime_disable(>dev);
 
return 0;
 }
-- 
2.17.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


[PATCHv4 2/4] watchdog: add support for adjusting last known HW keepalive time

2020-07-17 Thread Tero Kristo
Certain watchdogs require the watchdog only to be pinged within a
specific time window, pinging too early or too late cause the watchdog
to fire. In cases where this sort of watchdog has been started before
kernel comes up, we must adjust the watchdog keepalive window to match
the actually running timer, so add a new driver API for this purpose.

Signed-off-by: Tero Kristo 
---
 .../watchdog/watchdog-kernel-api.rst  | 12 
 drivers/watchdog/watchdog_dev.c   | 30 +++
 include/linux/watchdog.h  |  2 ++
 3 files changed, 44 insertions(+)

diff --git a/Documentation/watchdog/watchdog-kernel-api.rst 
b/Documentation/watchdog/watchdog-kernel-api.rst
index 068a55ee0d4a..baf44e986b07 100644
--- a/Documentation/watchdog/watchdog-kernel-api.rst
+++ b/Documentation/watchdog/watchdog-kernel-api.rst
@@ -336,3 +336,15 @@ an action is taken by a preconfigured pretimeout governor 
preassigned to
 the watchdog device. If watchdog pretimeout governor framework is not
 enabled, watchdog_notify_pretimeout() prints a notification message to
 the kernel log buffer.
+
+To set the last known HW keepalive time for a watchdog, the following function
+should be used::
+
+  int watchdog_set_last_hw_keepalive(struct watchdog_device *wdd,
+ unsigned int last_ping_ms)
+
+This function must be called immediately after watchdog registration. It
+sets the last known hardware heartbeat to have happened last_ping_ms before
+current time. Calling this is only needed if the watchdog is already running
+when probe is called, and the watchdog can only be pinged after the
+min_hw_heartbeat_ms time has passed from the last ping.
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index bc1cfa288553..e74a0c6811b5 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -1138,6 +1138,36 @@ void watchdog_dev_unregister(struct watchdog_device *wdd)
watchdog_cdev_unregister(wdd);
 }
 
+/*
+ * watchdog_set_last_hw_keepalive: set last HW keepalive time for watchdog
+ * @wdd: watchdog device
+ * @last_ping_ms: time since last HW heartbeat
+ *
+ * Adjusts the last known HW keepalive time for a watchdog timer.
+ * This is needed if the watchdog is already running when the probe
+ * function is called, and it can't be pinged immediately. This
+ * function must be called immediately after watchdog registration,
+ * and min_hw_heartbeat_ms must be set for this to be useful.
+ */
+int watchdog_set_last_hw_keepalive(struct watchdog_device *wdd,
+  unsigned int last_ping_ms)
+{
+   struct watchdog_core_data *wd_data;
+   ktime_t now;
+
+   if (!wdd)
+   return -EINVAL;
+
+   wd_data = wdd->wd_data;
+
+   now = ktime_get();
+
+   wd_data->last_hw_keepalive = ktime_sub(now, ms_to_ktime(last_ping_ms));
+
+   return __watchdog_ping(wdd);
+}
+EXPORT_SYMBOL_GPL(watchdog_set_last_hw_keepalive);
+
 /*
  * watchdog_dev_init: init dev part of watchdog core
  *
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 1464ce6ffa31..9b19e6bb68b5 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -210,6 +210,8 @@ extern int watchdog_init_timeout(struct watchdog_device 
*wdd,
 extern int watchdog_register_device(struct watchdog_device *);
 extern void watchdog_unregister_device(struct watchdog_device *);
 
+int watchdog_set_last_hw_keepalive(struct watchdog_device *, unsigned int);
+
 /* devres register variant */
 int devm_watchdog_register_device(struct device *dev, struct watchdog_device 
*);
 
-- 
2.17.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


[PATCHv4 3/4] watchdog: rti-wdt: attach to running watchdog during probe

2020-07-17 Thread Tero Kristo
If the RTI watchdog is running already during probe, the driver must
configure itself to match the HW. Window size and timeout is probed from
hardware, and the last keepalive ping is adjusted to match it also.

Signed-off-by: Tero Kristo 
---
 drivers/watchdog/rti_wdt.c | 112 +
 1 file changed, 102 insertions(+), 10 deletions(-)

diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
index d456dd72d99a..7cbdc178ffe8 100644
--- a/drivers/watchdog/rti_wdt.c
+++ b/drivers/watchdog/rti_wdt.c
@@ -35,7 +35,11 @@
 
 #define RTIWWDRX_NMI   0xa
 
-#define RTIWWDSIZE_50P 0x50
+#define RTIWWDSIZE_50P 0x50
+#define RTIWWDSIZE_25P 0x500
+#define RTIWWDSIZE_12P50x5000
+#define RTIWWDSIZE_6P250x5
+#define RTIWWDSIZE_3P125   0x50
 
 #define WDENABLE_KEY   0xa98559da
 
@@ -48,7 +52,7 @@
 
 #define DWDST  BIT(1)
 
-static int heartbeat;
+static int heartbeat = DEFAULT_HEARTBEAT;
 
 /*
  * struct to hold data for each WDT device
@@ -79,11 +83,9 @@ static int rti_wdt_start(struct watchdog_device *wdd)
 * be petted during the open window; not too early or not too late.
 * The HW configuration options only allow for the open window size
 * to be 50% or less than that; we obviouly want to configure the open
-* window as large as possible so we select the 50% option. To avoid
-* any glitches, we accommodate 5% safety margin also, so we setup
-* the min_hw_hearbeat at 55% of the timeout period.
+* window as large as possible so we select the 50% option.
 */
-   wdd->min_hw_heartbeat_ms = 11 * wdd->timeout * 1000 / 20;
+   wdd->min_hw_heartbeat_ms = 500 * wdd->timeout;
 
/* Generate NMI when wdt expires */
writel_relaxed(RTIWWDRX_NMI, wdt->base + RTIWWDRXCTRL);
@@ -110,7 +112,48 @@ static int rti_wdt_ping(struct watchdog_device *wdd)
return 0;
 }
 
-static unsigned int rti_wdt_get_timeleft(struct watchdog_device *wdd)
+static int rti_wdt_setup_hw_hb(struct watchdog_device *wdd, u32 wsize)
+{
+   /*
+* RTI only supports a windowed mode, where the watchdog can only
+* be petted during the open window; not too early or not too late.
+* The HW configuration options only allow for the open window size
+* to be 50% or less than that.
+*/
+   switch (wsize) {
+   case RTIWWDSIZE_50P:
+   /* 50% open window => 50% min heartbeat */
+   wdd->min_hw_heartbeat_ms = 500 * heartbeat;
+   break;
+
+   case RTIWWDSIZE_25P:
+   /* 25% open window => 75% min heartbeat */
+   wdd->min_hw_heartbeat_ms = 750 * heartbeat;
+   break;
+
+   case RTIWWDSIZE_12P5:
+   /* 12.5% open window => 87.5% min heartbeat */
+   wdd->min_hw_heartbeat_ms = 875 * heartbeat;
+   break;
+
+   case RTIWWDSIZE_6P25:
+   /* 6.5% open window => 93.5% min heartbeat */
+   wdd->min_hw_heartbeat_ms = 935 * heartbeat;
+   break;
+
+   case RTIWWDSIZE_3P125:
+   /* 3.125% open window => 96.9% min heartbeat */
+   wdd->min_hw_heartbeat_ms = 969 * heartbeat;
+   break;
+
+   default:
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
+static unsigned int rti_wdt_get_timeleft_ms(struct watchdog_device *wdd)
 {
u64 timer_counter;
u32 val;
@@ -123,11 +166,18 @@ static unsigned int rti_wdt_get_timeleft(struct 
watchdog_device *wdd)
 
timer_counter = readl_relaxed(wdt->base + RTIDWDCNTR);
 
+   timer_counter *= 1000;
+
do_div(timer_counter, wdt->freq);
 
return timer_counter;
 }
 
+static unsigned int rti_wdt_get_timeleft(struct watchdog_device *wdd)
+{
+   return rti_wdt_get_timeleft_ms(wdd) / 1000;
+}
+
 static const struct watchdog_info rti_wdt_info = {
.options = WDIOF_KEEPALIVEPING,
.identity = "K3 RTI Watchdog",
@@ -148,6 +198,7 @@ static int rti_wdt_probe(struct platform_device *pdev)
struct watchdog_device *wdd;
struct rti_wdt_device *wdt;
struct clk *clk;
+   u32 last_ping = 0;
 
wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
if (!wdt)
@@ -169,6 +220,14 @@ static int rti_wdt_probe(struct platform_device *pdev)
return -EINVAL;
}
 
+   /*
+* If watchdog is running at 32k clock, it is not accurate.
+* Adjust frequency down in this case so that we don't pet
+* the watchdog too often.
+*/
+   if (wdt->freq < 32768)
+   wdt->freq = wdt->freq * 9 / 10;
+
pm_runtime_enable(dev);
ret = pm_runtime_get_sync(dev);
if (ret) {
@@ -185,11 +244,8 @@ static int rti_wdt_probe(struct platform_device *pdev)
wdd->min_timeout =

Re: [PATCH 7/7] arm64: defconfig: Enable AM654x SDHCI controller

2020-07-17 Thread Tero Kristo

On 17/07/2020 11:38, Faiz Abbas wrote:

Hi,

On 16/07/20 11:58 pm, Arnd Bergmann wrote:

On Thu, Jul 16, 2020 at 3:25 PM Sekhar Nori  wrote:


On 7/16/20 5:49 PM, Faiz Abbas wrote:

Hi,

On 19/06/20 6:28 pm, Faiz Abbas wrote:

Enable CONFIG_SDHCI_AM654 to Support AM65x sdhci controller.

Signed-off-by: Faiz Abbas 
---
  arch/arm64/configs/defconfig | 1 +
  1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 883e8bace3ed..40dd13e0adc5 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -731,6 +731,7 @@ CONFIG_MMC_DW_ROCKCHIP=y
  CONFIG_MMC_SUNXI=y
  CONFIG_MMC_BCM2835=y
  CONFIG_MMC_SDHCI_XENON=y
+CONFIG_MMC_SDHCI_AM654=y
  CONFIG_MMC_OWL=y
  CONFIG_NEW_LEDS=y
  CONFIG_LEDS_CLASS=y



Gentle ping. Will, Catalin, can this patch be picked up?


 From logs, Arnd has been picking up patches for this file. Looping in
Arnd and ARM-SoC team.


I tend to ignore individual patches to the defconfig file unless
they are sent to:s...@kernel.org. The best way to get them
included is to have the platform maintainers pick up the
changes and send them that way as a separate pull request
at the same time as sending any DT updates.

The MAINTAINERS file lists Tero and Nishanth as maintainers
for the platform. If they want, I can apply this one directly, but in
the future, send it to them.



Thanks for clarifying Arnd. Tero, can you pick this up?


Ok, this topic has been bit unclear for me also, but if you say I can 
pick the patches myself and send a pull request out, I can do that.


-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH v4 0/6] arm64: ti: k3-j721e: Add SERDES PHY and USB3.0 support

2020-07-17 Thread Tero Kristo

On 29/06/2020 15:52, Roger Quadros wrote:

Hi Tero,

This series adds SERDES PHY support and Type-C USB Super-Speed support
to the J721E EVM.

Please queue this for -next. Thanks.


Queued up for 5.9, thanks.

-Tero



cheers,
-roger

Changelog:
v4:
- Removed redundant patch
- used compaible string for yaml filename
- typo fix s/mdf/mfd in patch subject
- added simple-mfd, address-cells, size-cells and ranges

v3:
- Add new DT schema for J721E System controller.
- Re-order system controller's compatible string i.e. most compatible to least.

v2:
- Addressed Rob's comments.
- Changed type-C debounce delay from 300ms to 700ms as 300ms is not
sufficient on EVM.


Kishon Vijay Abraham I (2):
   arm64: dts: ti: k3-j721e-main: Add WIZ and SERDES PHY nodes
   arm64: dts: ti: k3-j721e-main: Add system controller node and SERDES
 lane mux

Roger Quadros (4):
   dt-bindings: mfd: ti,j721e-system-controller.yaml: Add J721e system
 controller
   arm64: dts: ti: k3-j721e-main.dtsi: Add USB to SERDES MUX
   arm64: dts: ti: k3-j721e: Enable Super-Speed support for USB0
   arm64: dts: k3-j721e-proc-board: Add wait time for sampling Type-C DIR
 line

  .../mfd/ti,j721e-system-controller.yaml   |  74 +
  .../dts/ti/k3-j721e-common-proc-board.dts |  33 ++-
  arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 275 ++
  include/dt-bindings/mux/mux-j721e-wiz.h   |  53 
  4 files changed, 433 insertions(+), 2 deletions(-)
  create mode 100644 
Documentation/devicetree/bindings/mfd/ti,j721e-system-controller.yaml
  create mode 100644 include/dt-bindings/mux/mux-j721e-wiz.h



--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH] arm64: dts: ti: k3-am65/j721e-main: rename gic-its node to msi-controller

2020-07-17 Thread Tero Kristo

On 26/06/2020 21:34, Grygorii Strashko wrote:

The preferable name for gic-its is msi-controller, so rename it to fix
dtbs_check warning:

k3-j721e-common-proc-board.dt.yaml: interrupt-controller@180:
gic-its@182: False schema does not allow {'compatible':
['arm,gic-v3-its'], 'reg': [[0, 25296896, 0, 65536]],
'socionext,synquacer-pre-its': [[16777216, 4194304]], 'msi-controller':
True, '#msi-cells': [[1]]}

Signed-off-by: Grygorii Strashko 


Queued up for 5.9, thanks.

-Tero


---
  arch/arm64/boot/dts/ti/k3-am65-main.dtsi  | 2 +-
  arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi 
b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
index 61815228e230..d14e0c65d266 100644
--- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
@@ -42,7 +42,7 @@
 */
interrupts = ;
  
-		gic_its: gic-its@182 {

+   gic_its: msi-controller@182 {
compatible = "arm,gic-v3-its";
reg = <0x00 0x0182 0x00 0x1>;
socionext,synquacer-pre-its = <0x100 0x40>;
diff --git a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi 
b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
index 96c929da639d..5d82de4097bb 100644
--- a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
@@ -31,7 +31,7 @@
/* vcpumntirq: virtual CPU interface maintenance interrupt */
interrupts = ;
  
-		gic_its: gic-its@182 {

+   gic_its: msi-controller@182 {
compatible = "arm,gic-v3-its";
reg = <0x00 0x0182 0x00 0x1>;
socionext,synquacer-pre-its = <0x100 0x40>;



--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH 3/3] arm64: dts: ti: k3-j721e-main: rename smmu node to iommu

2020-07-17 Thread Tero Kristo

On 26/06/2020 21:40, Suman Anna wrote:

On 6/26/20 1:36 PM, Grygorii Strashko wrote:



On 26/06/2020 21:35, Grygorii Strashko wrote:

Rename smmu node to iommu to fix dtbs_check warning:
  k3-j721e-common-proc-board.dt.yaml: smmu@3660: $nodename:0: 
'smmu@3660' does not match '^iommu@[0-9a-f]*'


Signed-off-by: Grygorii Strashko 


Acked-by: Suman Anna 


Queued up for 5.9, thanks.

-Tero




---
  arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi 
b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi

index 5d82de4097bb..0ac23c4414a2 100644
--- a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
@@ -95,7 +95,7 @@
  interrupts = ;
  };
-    smmu0: smmu@3660 {
+    smmu0: iommu@3660 {
  compatible = "arm,smmu-v3";
  reg = <0x0 0x3660 0x0 0x10>;
  interrupt-parent = <>;



Pls, ignore "3/3" in subj.





--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH] arm64: dts: ti: k3-*: Replace HTTP links with HTTPS ones

2020-07-17 Thread Tero Kristo

On 13/07/2020 13:14, Alexander A. Klimov wrote:

Rationale:
Reduces attack surface on kernel devs opening the links for MITM
as HTTPS traffic is much harder to manipulate.

Deterministic algorithm:
For each file:
   If not .svg:
 For each line:
   If doesn't contain `\bxmlns\b`:
 For each link, `\bhttp://[^# \t\r\n]*(?:\w|/)`:
  If neither `\bgnu\.org/license`, nor `\bmozilla\.org/MPL\b`:
 If both the HTTP and HTTPS versions
 return 200 OK and serve the same content:
   Replace HTTP with HTTPS.

Signed-off-by: Alexander A. Klimov 


Queued up for 5.9, thanks.

-Tero


---
  Continuing my work started at 93431e0607e5.
  See also: git log --oneline '--author=Alexander A. Klimov 
' v5.7..master
  (Actually letting a shell for loop submit all this stuff for me.)

  If there are any URLs to be removed completely or at least not just 
HTTPSified:
  Just clearly say so and I'll *undo my change*.
  See also: https://lkml.org/lkml/2020/6/27/64

  If there are any valid, but yet not changed URLs:
  See: https://lkml.org/lkml/2020/6/26/837

  If you apply the patch, please let me know.

  Sorry again to all maintainers who complained about subject lines.
  Now I realized that you want an actually perfect prefixes,
  not just subsystem ones.
  I tried my best...
  And yes, *I could* (at least half-)automate it.
  Impossible is nothing! :)


  arch/arm64/boot/dts/ti/Makefile   | 2 +-
  arch/arm64/boot/dts/ti/k3-am65-main.dtsi  | 2 +-
  arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi   | 2 +-
  arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi| 2 +-
  arch/arm64/boot/dts/ti/k3-am65.dtsi   | 2 +-
  arch/arm64/boot/dts/ti/k3-am654-base-board.dts| 2 +-
  arch/arm64/boot/dts/ti/k3-am654.dtsi  | 2 +-
  arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts | 2 +-
  arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 2 +-
  arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi   | 2 +-
  arch/arm64/boot/dts/ti/k3-j721e-som-p0.dtsi   | 2 +-
  arch/arm64/boot/dts/ti/k3-j721e.dtsi  | 2 +-
  include/dt-bindings/pinctrl/k3.h  | 2 +-
  13 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/Makefile b/arch/arm64/boot/dts/ti/Makefile
index b397945fdf73..05c0bebf65d4 100644
--- a/arch/arm64/boot/dts/ti/Makefile
+++ b/arch/arm64/boot/dts/ti/Makefile
@@ -3,7 +3,7 @@
  # Make file to build device tree binaries for boards based on
  # Texas Instruments Inc processors
  #
-# Copyright (C) 2016-2018 Texas Instruments Incorporated - http://www.ti.com/
+# Copyright (C) 2016-2018 Texas Instruments Incorporated - https://www.ti.com/
  #
  
  dtb-$(CONFIG_ARCH_K3_AM6_SOC) += k3-am654-base-board.dtb

diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi 
b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
index 61815228e230..940acc6cbf26 100644
--- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
@@ -2,7 +2,7 @@
  /*
   * Device Tree Source for AM6 SoC Family Main Domain peripherals
   *
- * Copyright (C) 2016-2018 Texas Instruments Incorporated - http://www.ti.com/
+ * Copyright (C) 2016-2018 Texas Instruments Incorporated - https://www.ti.com/
   */
  #include 
  
diff --git a/arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi b/arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi

index ae5f813d0cac..8c1abcfe0860 100644
--- a/arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi
@@ -2,7 +2,7 @@
  /*
   * Device Tree Source for AM6 SoC Family MCU Domain peripherals
   *
- * Copyright (C) 2016-2018 Texas Instruments Incorporated - http://www.ti.com/
+ * Copyright (C) 2016-2018 Texas Instruments Incorporated - https://www.ti.com/
   */
  
  _mcu {

diff --git a/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi 
b/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi
index 54a133fa1bf2..dd75a5592539 100644
--- a/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi
@@ -2,7 +2,7 @@
  /*
   * Device Tree Source for AM6 SoC Family Wakeup Domain peripherals
   *
- * Copyright (C) 2016-2018 Texas Instruments Incorporated - http://www.ti.com/
+ * Copyright (C) 2016-2018 Texas Instruments Incorporated - https://www.ti.com/
   */
  
  _wakeup {

diff --git a/arch/arm64/boot/dts/ti/k3-am65.dtsi 
b/arch/arm64/boot/dts/ti/k3-am65.dtsi
index 5be75e430965..27c0406b10ba 100644
--- a/arch/arm64/boot/dts/ti/k3-am65.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65.dtsi
@@ -2,7 +2,7 @@
  /*
   * Device Tree Source for AM6 SoC Family
   *
- * Copyright (C) 2016-2018 Texas Instruments Incorporated - http://www.ti.com/
+ * Copyright (C) 2016-2018 Texas Instruments Incorporated - https://www.ti.com/
   */
  
  #include 

diff --git a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts 
b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
index 2f3d3316a1cf..78084a507fcf 100644
--- 

Re: [PATCH 0/2] Add support for SD card on on AM65x-evm

2020-07-17 Thread Tero Kristo

On 10/07/2020 22:02, Faiz Abbas wrote:

The following patches add support for SD card node in am654x-evm

Because of fundamental interface issues (see patch 2 for details),
SD card was never enabled for silicon revision 1.0

These issues have been fixed with SR2.0 but boards with SR1.0 are
recommended to disable this node

These patches depend on kernel patches for supporting silicon revision
2.0 posted here:
https://patchwork.kernel.org/project/linux-mmc/list/?series=305565

The dependencies have been picked up and are in linux-next


Queued up for 5.9, thanks.

-Tero



Faiz Abbas (2):
   arm64: dts: ti: k3-am65-main: Add support for sdhci1
   arm64: dts: ti: k3-am654-base-board: Add support for SD card

  arch/arm64/boot/dts/ti/k3-am65-main.dtsi  | 24 +++
  .../arm64/boot/dts/ti/k3-am654-base-board.dts | 24 +++
  2 files changed, 48 insertions(+)



--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH v2 0/2] arm64: dts: ti: k3-j721e-common-proc-board: Enable audio support

2020-07-17 Thread Tero Kristo

On 03/07/2020 10:44, Peter Ujfalusi wrote:

Hi,

Change since v1:
- not including dt-bindings/sound/ti-mcasp.h as it is not needed

the DT binding document and the driver is now in linux-next:
https://lore.kernel.org/lkml/159364215574.10630.2058528286314798186.b4...@kernel.org/

Before adding the audio support, first fix up the DTS file by removing the
duplicated main_i2c1_exp4_pins_default.

Regards,
Peter


Queued up for 5.9, thanks.

-Tero


---
Peter Ujfalusi (2):
   arm64: dts: ti: k3-j721e-common-proc-board: Remove duplicated
 main_i2c1_exp4_pins_default
   arm64: dts: ti: j721e-common-proc-board: Analog audio support

  .../dts/ti/k3-j721e-common-proc-board.dts | 136 +-
  1 file changed, 133 insertions(+), 3 deletions(-)



--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH] Replace HTTP links with HTTPS ones: TI KEYSTONE MULTICORE NAVIGATOR DRIVERS

2020-07-17 Thread Tero Kristo

On 08/07/2020 21:19, Alexander A. Klimov wrote:

Rationale:
Reduces attack surface on kernel devs opening the links for MITM
as HTTPS traffic is much harder to manipulate.

Deterministic algorithm:
For each file:
   If not .svg:
 For each line:
   If doesn't contain `\bxmlns\b`:
 For each link, `\bhttp://[^# \t\r\n]*(?:\w|/)`:
  If neither `\bgnu\.org/license`, nor `\bmozilla\.org/MPL\b`:
 If both the HTTP and HTTPS versions
 return 200 OK and serve the same content:
   Replace HTTP with HTTPS.

Signed-off-by: Alexander A. Klimov 


Acked-by: Tero Kristo 

Santosh, are you going to pick this up?

-Tero


---
  Continuing my work started at 93431e0607e5.
  See also: git log --oneline '--author=Alexander A. Klimov 
' v5.7..master
  (Actually letting a shell for loop submit all this stuff for me.)

  If there are any URLs to be removed completely or at least not HTTPSified:
  Just clearly say so and I'll *undo my change*.
  See also: https://lkml.org/lkml/2020/6/27/64

  If there are any valid, but yet not changed URLs:
  See: https://lkml.org/lkml/2020/6/26/837

  If you apply the patch, please let me know.


  drivers/soc/ti/k3-ringacc.c| 2 +-
  drivers/soc/ti/knav_qmss.h | 2 +-
  drivers/soc/ti/knav_qmss_acc.c | 2 +-
  drivers/soc/ti/knav_qmss_queue.c   | 2 +-
  drivers/soc/ti/omap_prm.c  | 2 +-
  drivers/soc/ti/pm33xx.c| 2 +-
  drivers/soc/ti/ti_sci_inta_msi.c   | 2 +-
  drivers/soc/ti/ti_sci_pm_domains.c | 2 +-
  8 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/soc/ti/k3-ringacc.c b/drivers/soc/ti/k3-ringacc.c
index 5fb2ee2ac978..036d89aa3e2b 100644
--- a/drivers/soc/ti/k3-ringacc.c
+++ b/drivers/soc/ti/k3-ringacc.c
@@ -2,7 +2,7 @@
  /*
   * TI K3 NAVSS Ring Accelerator subsystem driver
   *
- * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com
+ * Copyright (C) 2019 Texas Instruments Incorporated - https://www.ti.com
   */
  
  #include 

diff --git a/drivers/soc/ti/knav_qmss.h b/drivers/soc/ti/knav_qmss.h
index a01eda720bf6..b815aed8f365 100644
--- a/drivers/soc/ti/knav_qmss.h
+++ b/drivers/soc/ti/knav_qmss.h
@@ -2,7 +2,7 @@
  /*
   * Keystone Navigator QMSS driver internal header
   *
- * Copyright (C) 2014 Texas Instruments Incorporated - http://www.ti.com
+ * Copyright (C) 2014 Texas Instruments Incorporated - https://www.ti.com
   * Author:Sandeep Nair 
   *Cyril Chemparathy 
   *Santosh Shilimkar 
diff --git a/drivers/soc/ti/knav_qmss_acc.c b/drivers/soc/ti/knav_qmss_acc.c
index 1762d89fc05d..29670d47cdfd 100644
--- a/drivers/soc/ti/knav_qmss_acc.c
+++ b/drivers/soc/ti/knav_qmss_acc.c
@@ -2,7 +2,7 @@
  /*
   * Keystone accumulator queue manager
   *
- * Copyright (C) 2014 Texas Instruments Incorporated - http://www.ti.com
+ * Copyright (C) 2014 Texas Instruments Incorporated - https://www.ti.com
   * Author:Sandeep Nair 
   *Cyril Chemparathy 
   *Santosh Shilimkar 
diff --git a/drivers/soc/ti/knav_qmss_queue.c b/drivers/soc/ti/knav_qmss_queue.c
index aa071d96ef36..feaead96ff93 100644
--- a/drivers/soc/ti/knav_qmss_queue.c
+++ b/drivers/soc/ti/knav_qmss_queue.c
@@ -2,7 +2,7 @@
  /*
   * Keystone Queue Manager subsystem driver
   *
- * Copyright (C) 2014 Texas Instruments Incorporated - http://www.ti.com
+ * Copyright (C) 2014 Texas Instruments Incorporated - https://www.ti.com
   * Authors:   Sandeep Nair 
   *Cyril Chemparathy 
   *Santosh Shilimkar 
diff --git a/drivers/soc/ti/omap_prm.c b/drivers/soc/ti/omap_prm.c
index c9b3f9ebf0bb..06e9d59e75bf 100644
--- a/drivers/soc/ti/omap_prm.c
+++ b/drivers/soc/ti/omap_prm.c
@@ -2,7 +2,7 @@
  /*
   * OMAP2+ PRM driver
   *
- * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
+ * Copyright (C) 2019 Texas Instruments Incorporated - https://www.ti.com/
   *Tero Kristo 
   */
  
diff --git a/drivers/soc/ti/pm33xx.c b/drivers/soc/ti/pm33xx.c

index de0123ec8ad6..9bd2da2758f4 100644
--- a/drivers/soc/ti/pm33xx.c
+++ b/drivers/soc/ti/pm33xx.c
@@ -2,7 +2,7 @@
  /*
   * AM33XX Power Management Routines
   *
- * Copyright (C) 2012-2018 Texas Instruments Incorporated - http://www.ti.com/
+ * Copyright (C) 2012-2018 Texas Instruments Incorporated - https://www.ti.com/
   *Vaibhav Bedia, Dave Gerlach
   */
  
diff --git a/drivers/soc/ti/ti_sci_inta_msi.c b/drivers/soc/ti/ti_sci_inta_msi.c

index 0eb9462f609e..8e439759b451 100644
--- a/drivers/soc/ti/ti_sci_inta_msi.c
+++ b/drivers/soc/ti/ti_sci_inta_msi.c
@@ -2,7 +2,7 @@
  /*
   * Texas Instruments' K3 Interrupt Aggregator MSI bus
   *
- * Copyright (C) 2018-2019 Texas Instruments Incorporated - http://www.ti.com/
+ * Copyright (C) 2018-2019 Texas Instruments Incorporated - https://www.ti.com/
   *Lokesh Vutla 
   */
  
diff --git a/drivers/soc/ti/ti_sci_pm_domains.c b/drivers/soc/ti/ti_sci_pm_domains.c

index 8c2a2f23982c..771b0ab51872 100644
--- a/drivers/soc

Re: [PATCHv3 3/4] watchdog: rti-wdt: attach to running watchdog during probe

2020-07-14 Thread Tero Kristo

On 14/07/2020 10:06, Guenter Roeck wrote:

On 7/13/20 11:50 PM, Tero Kristo wrote:

On 14/07/2020 08:15, Guenter Roeck wrote:

On 7/13/20 6:18 AM, Tero Kristo wrote:

If the RTI watchdog is running already during probe, the driver must
configure itself to match the HW. Window size and timeout is probed from
hardware, and the last keepalive ping is adjusted to match it also.

Signed-off-by: Tero Kristo 
---
   drivers/watchdog/rti_wdt.c | 111 +
   1 file changed, 101 insertions(+), 10 deletions(-)

diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
index d456dd72d99a..45dfc546e371 100644
--- a/drivers/watchdog/rti_wdt.c
+++ b/drivers/watchdog/rti_wdt.c
@@ -35,7 +35,11 @@
     #define RTIWWDRX_NMI    0xa
   -#define RTIWWDSIZE_50P    0x50
+#define RTIWWDSIZE_50P    0x50
+#define RTIWWDSIZE_25P    0x500
+#define RTIWWDSIZE_12P5    0x5000
+#define RTIWWDSIZE_6P25    0x5
+#define RTIWWDSIZE_3P125    0x50
     #define WDENABLE_KEY    0xa98559da
   @@ -48,7 +52,7 @@
     #define DWDST    BIT(1)
   -static int heartbeat;
+static int heartbeat = DEFAULT_HEARTBEAT;
     /*
    * struct to hold data for each WDT device
@@ -79,11 +83,9 @@ static int rti_wdt_start(struct watchdog_device *wdd)
    * be petted during the open window; not too early or not too late.
    * The HW configuration options only allow for the open window size
    * to be 50% or less than that; we obviouly want to configure the open
- * window as large as possible so we select the 50% option. To avoid
- * any glitches, we accommodate 5% safety margin also, so we setup
- * the min_hw_hearbeat at 55% of the timeout period.
+ * window as large as possible so we select the 50% option.
    */
-    wdd->min_hw_heartbeat_ms = 11 * wdd->timeout * 1000 / 20;
+    wdd->min_hw_heartbeat_ms = 500 * wdd->timeout;
     /* Generate NMI when wdt expires */
   writel_relaxed(RTIWWDRX_NMI, wdt->base + RTIWWDRXCTRL);
@@ -110,7 +112,48 @@ static int rti_wdt_ping(struct watchdog_device *wdd)
   return 0;
   }
   -static unsigned int rti_wdt_get_timeleft(struct watchdog_device *wdd)
+static int rti_wdt_setup_hw_hb(struct watchdog_device *wdd, u32 wsize)
+{
+    /*
+ * RTI only supports a windowed mode, where the watchdog can only
+ * be petted during the open window; not too early or not too late.
+ * The HW configuration options only allow for the open window size
+ * to be 50% or less than that.
+ */
+    switch (wsize) {
+    case RTIWWDSIZE_50P:
+    /* 50% open window => 50% min heartbeat */
+    wdd->min_hw_heartbeat_ms = 500 * heartbeat;
+    break;
+
+    case RTIWWDSIZE_25P:
+    /* 25% open window => 75% min heartbeat */
+    wdd->min_hw_heartbeat_ms = 750 * heartbeat;
+    break;
+
+    case RTIWWDSIZE_12P5:
+    /* 12.5% open window => 87.5% min heartbeat */
+    wdd->min_hw_heartbeat_ms = 875 * heartbeat;
+    break;
+
+    case RTIWWDSIZE_6P25:
+    /* 6.5% open window => 93.5% min heartbeat */
+    wdd->min_hw_heartbeat_ms = 935 * heartbeat;
+    break;
+
+    case RTIWWDSIZE_3P125:
+    /* 3.125% open window => 96.9% min heartbeat */
+    wdd->min_hw_heartbeat_ms = 969 * heartbeat;
+    break;
+
+    default:
+    return -EINVAL;
+    }
+
+    return 0;
+}
+
+static unsigned int rti_wdt_get_timeleft_ms(struct watchdog_device *wdd)
   {
   u64 timer_counter;
   u32 val;
@@ -123,11 +166,18 @@ static unsigned int rti_wdt_get_timeleft(struct 
watchdog_device *wdd)
     timer_counter = readl_relaxed(wdt->base + RTIDWDCNTR);
   +    timer_counter *= 1000;
+
   do_div(timer_counter, wdt->freq);
     return timer_counter;
   }
   +static unsigned int rti_wdt_get_timeleft(struct watchdog_device *wdd)
+{
+    return rti_wdt_get_timeleft_ms(wdd) / 1000;
+}
+
   static const struct watchdog_info rti_wdt_info = {
   .options = WDIOF_KEEPALIVEPING,
   .identity = "K3 RTI Watchdog",
@@ -148,6 +198,7 @@ static int rti_wdt_probe(struct platform_device *pdev)
   struct watchdog_device *wdd;
   struct rti_wdt_device *wdt;
   struct clk *clk;
+    u32 last_ping = 0;
     wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
   if (!wdt)
@@ -169,6 +220,14 @@ static int rti_wdt_probe(struct platform_device *pdev)
   return -EINVAL;
   }
   +    /*
+ * If watchdog is running at 32k clock, it is not accurate.
+ * Adjust frequency down in this case so that we don't pet
+ * the watchdog too often.
+ */
+    if (wdt->freq < 32768)
+    wdt->freq = wdt->freq * 9 / 10;
+


So this is now only a problem is the window size was set restrictively
in the BOS/ROMMON. Meaning it is still a problem. How is this better than
before ? Why not just always set the window size to something reasonable ?


Re-programming of the watchd

Re: [PATCHv3 3/4] watchdog: rti-wdt: attach to running watchdog during probe

2020-07-14 Thread Tero Kristo

On 14/07/2020 08:15, Guenter Roeck wrote:

On 7/13/20 6:18 AM, Tero Kristo wrote:

If the RTI watchdog is running already during probe, the driver must
configure itself to match the HW. Window size and timeout is probed from
hardware, and the last keepalive ping is adjusted to match it also.

Signed-off-by: Tero Kristo 
---
  drivers/watchdog/rti_wdt.c | 111 +
  1 file changed, 101 insertions(+), 10 deletions(-)

diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
index d456dd72d99a..45dfc546e371 100644
--- a/drivers/watchdog/rti_wdt.c
+++ b/drivers/watchdog/rti_wdt.c
@@ -35,7 +35,11 @@
  
  #define RTIWWDRX_NMI	0xa
  
-#define RTIWWDSIZE_50P	0x50

+#define RTIWWDSIZE_50P 0x50
+#define RTIWWDSIZE_25P 0x500
+#define RTIWWDSIZE_12P50x5000
+#define RTIWWDSIZE_6P250x5
+#define RTIWWDSIZE_3P125   0x50
  
  #define WDENABLE_KEY	0xa98559da
  
@@ -48,7 +52,7 @@
  
  #define DWDST			BIT(1)
  
-static int heartbeat;

+static int heartbeat = DEFAULT_HEARTBEAT;
  
  /*

   * struct to hold data for each WDT device
@@ -79,11 +83,9 @@ static int rti_wdt_start(struct watchdog_device *wdd)
 * be petted during the open window; not too early or not too late.
 * The HW configuration options only allow for the open window size
 * to be 50% or less than that; we obviouly want to configure the open
-* window as large as possible so we select the 50% option. To avoid
-* any glitches, we accommodate 5% safety margin also, so we setup
-* the min_hw_hearbeat at 55% of the timeout period.
+* window as large as possible so we select the 50% option.
 */
-   wdd->min_hw_heartbeat_ms = 11 * wdd->timeout * 1000 / 20;
+   wdd->min_hw_heartbeat_ms = 500 * wdd->timeout;
  
  	/* Generate NMI when wdt expires */

writel_relaxed(RTIWWDRX_NMI, wdt->base + RTIWWDRXCTRL);
@@ -110,7 +112,48 @@ static int rti_wdt_ping(struct watchdog_device *wdd)
return 0;
  }
  
-static unsigned int rti_wdt_get_timeleft(struct watchdog_device *wdd)

+static int rti_wdt_setup_hw_hb(struct watchdog_device *wdd, u32 wsize)
+{
+   /*
+* RTI only supports a windowed mode, where the watchdog can only
+* be petted during the open window; not too early or not too late.
+* The HW configuration options only allow for the open window size
+* to be 50% or less than that.
+*/
+   switch (wsize) {
+   case RTIWWDSIZE_50P:
+   /* 50% open window => 50% min heartbeat */
+   wdd->min_hw_heartbeat_ms = 500 * heartbeat;
+   break;
+
+   case RTIWWDSIZE_25P:
+   /* 25% open window => 75% min heartbeat */
+   wdd->min_hw_heartbeat_ms = 750 * heartbeat;
+   break;
+
+   case RTIWWDSIZE_12P5:
+   /* 12.5% open window => 87.5% min heartbeat */
+   wdd->min_hw_heartbeat_ms = 875 * heartbeat;
+   break;
+
+   case RTIWWDSIZE_6P25:
+   /* 6.5% open window => 93.5% min heartbeat */
+   wdd->min_hw_heartbeat_ms = 935 * heartbeat;
+   break;
+
+   case RTIWWDSIZE_3P125:
+   /* 3.125% open window => 96.9% min heartbeat */
+   wdd->min_hw_heartbeat_ms = 969 * heartbeat;
+   break;
+
+   default:
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
+static unsigned int rti_wdt_get_timeleft_ms(struct watchdog_device *wdd)
  {
u64 timer_counter;
u32 val;
@@ -123,11 +166,18 @@ static unsigned int rti_wdt_get_timeleft(struct 
watchdog_device *wdd)
  
  	timer_counter = readl_relaxed(wdt->base + RTIDWDCNTR);
  
+	timer_counter *= 1000;

+
do_div(timer_counter, wdt->freq);
  
  	return timer_counter;

  }
  
+static unsigned int rti_wdt_get_timeleft(struct watchdog_device *wdd)

+{
+   return rti_wdt_get_timeleft_ms(wdd) / 1000;
+}
+
  static const struct watchdog_info rti_wdt_info = {
.options = WDIOF_KEEPALIVEPING,
.identity = "K3 RTI Watchdog",
@@ -148,6 +198,7 @@ static int rti_wdt_probe(struct platform_device *pdev)
struct watchdog_device *wdd;
struct rti_wdt_device *wdt;
struct clk *clk;
+   u32 last_ping = 0;
  
  	wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);

if (!wdt)
@@ -169,6 +220,14 @@ static int rti_wdt_probe(struct platform_device *pdev)
return -EINVAL;
}
  
+	/*

+* If watchdog is running at 32k clock, it is not accurate.
+* Adjust frequency down in this case so that we don't pet
+* the watchdog too often.
+*/
+   if (wdt->freq < 32768)
+   wdt->freq = wdt->freq * 9 / 10;
+


So this is now only a problem is the window size was set restrictively
in the BOS/ROMMON. Meaning it is still a problem.

[PATCHv3 2/4] watchdog: add support for adjusting last known HW keepalive time

2020-07-13 Thread Tero Kristo
Certain watchdogs require the watchdog only to be pinged within a
specific time window, pinging too early or too late cause the watchdog
to fire. In cases where this sort of watchdog has been started before
kernel comes up, we must adjust the watchdog keepalive window to match
the actually running timer, so add a new driver API for this purpose.

Signed-off-by: Tero Kristo 
---
 .../watchdog/watchdog-kernel-api.rst  | 12 
 drivers/watchdog/watchdog_dev.c   | 30 +++
 include/linux/watchdog.h  |  2 ++
 3 files changed, 44 insertions(+)

diff --git a/Documentation/watchdog/watchdog-kernel-api.rst 
b/Documentation/watchdog/watchdog-kernel-api.rst
index 068a55ee0d4a..baf44e986b07 100644
--- a/Documentation/watchdog/watchdog-kernel-api.rst
+++ b/Documentation/watchdog/watchdog-kernel-api.rst
@@ -336,3 +336,15 @@ an action is taken by a preconfigured pretimeout governor 
preassigned to
 the watchdog device. If watchdog pretimeout governor framework is not
 enabled, watchdog_notify_pretimeout() prints a notification message to
 the kernel log buffer.
+
+To set the last known HW keepalive time for a watchdog, the following function
+should be used::
+
+  int watchdog_set_last_hw_keepalive(struct watchdog_device *wdd,
+ unsigned int last_ping_ms)
+
+This function must be called immediately after watchdog registration. It
+sets the last known hardware heartbeat to have happened last_ping_ms before
+current time. Calling this is only needed if the watchdog is already running
+when probe is called, and the watchdog can only be pinged after the
+min_hw_heartbeat_ms time has passed from the last ping.
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index bc1cfa288553..e74a0c6811b5 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -1138,6 +1138,36 @@ void watchdog_dev_unregister(struct watchdog_device *wdd)
watchdog_cdev_unregister(wdd);
 }
 
+/*
+ * watchdog_set_last_hw_keepalive: set last HW keepalive time for watchdog
+ * @wdd: watchdog device
+ * @last_ping_ms: time since last HW heartbeat
+ *
+ * Adjusts the last known HW keepalive time for a watchdog timer.
+ * This is needed if the watchdog is already running when the probe
+ * function is called, and it can't be pinged immediately. This
+ * function must be called immediately after watchdog registration,
+ * and min_hw_heartbeat_ms must be set for this to be useful.
+ */
+int watchdog_set_last_hw_keepalive(struct watchdog_device *wdd,
+  unsigned int last_ping_ms)
+{
+   struct watchdog_core_data *wd_data;
+   ktime_t now;
+
+   if (!wdd)
+   return -EINVAL;
+
+   wd_data = wdd->wd_data;
+
+   now = ktime_get();
+
+   wd_data->last_hw_keepalive = ktime_sub(now, ms_to_ktime(last_ping_ms));
+
+   return __watchdog_ping(wdd);
+}
+EXPORT_SYMBOL_GPL(watchdog_set_last_hw_keepalive);
+
 /*
  * watchdog_dev_init: init dev part of watchdog core
  *
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 1464ce6ffa31..9b19e6bb68b5 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -210,6 +210,8 @@ extern int watchdog_init_timeout(struct watchdog_device 
*wdd,
 extern int watchdog_register_device(struct watchdog_device *);
 extern void watchdog_unregister_device(struct watchdog_device *);
 
+int watchdog_set_last_hw_keepalive(struct watchdog_device *, unsigned int);
+
 /* devres register variant */
 int devm_watchdog_register_device(struct device *dev, struct watchdog_device 
*);
 
-- 
2.17.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


[PATCHv3 0/4] watchdog: rti: support attach to running timer

2020-07-13 Thread Tero Kristo
Hi,

Changes from previous version:

- Documentation changes for patch #2
- Dropped the configurable module parameter for window size
- Merged any needed functionality from old patches #3 and #4 to patch #3
  now
- Added new rti driver internal API for getting remaining milliseconds
  left on the timer

-Tero


--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


[PATCHv3 3/4] watchdog: rti-wdt: attach to running watchdog during probe

2020-07-13 Thread Tero Kristo
If the RTI watchdog is running already during probe, the driver must
configure itself to match the HW. Window size and timeout is probed from
hardware, and the last keepalive ping is adjusted to match it also.

Signed-off-by: Tero Kristo 
---
 drivers/watchdog/rti_wdt.c | 111 +
 1 file changed, 101 insertions(+), 10 deletions(-)

diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
index d456dd72d99a..45dfc546e371 100644
--- a/drivers/watchdog/rti_wdt.c
+++ b/drivers/watchdog/rti_wdt.c
@@ -35,7 +35,11 @@
 
 #define RTIWWDRX_NMI   0xa
 
-#define RTIWWDSIZE_50P 0x50
+#define RTIWWDSIZE_50P 0x50
+#define RTIWWDSIZE_25P 0x500
+#define RTIWWDSIZE_12P50x5000
+#define RTIWWDSIZE_6P250x5
+#define RTIWWDSIZE_3P125   0x50
 
 #define WDENABLE_KEY   0xa98559da
 
@@ -48,7 +52,7 @@
 
 #define DWDST  BIT(1)
 
-static int heartbeat;
+static int heartbeat = DEFAULT_HEARTBEAT;
 
 /*
  * struct to hold data for each WDT device
@@ -79,11 +83,9 @@ static int rti_wdt_start(struct watchdog_device *wdd)
 * be petted during the open window; not too early or not too late.
 * The HW configuration options only allow for the open window size
 * to be 50% or less than that; we obviouly want to configure the open
-* window as large as possible so we select the 50% option. To avoid
-* any glitches, we accommodate 5% safety margin also, so we setup
-* the min_hw_hearbeat at 55% of the timeout period.
+* window as large as possible so we select the 50% option.
 */
-   wdd->min_hw_heartbeat_ms = 11 * wdd->timeout * 1000 / 20;
+   wdd->min_hw_heartbeat_ms = 500 * wdd->timeout;
 
/* Generate NMI when wdt expires */
writel_relaxed(RTIWWDRX_NMI, wdt->base + RTIWWDRXCTRL);
@@ -110,7 +112,48 @@ static int rti_wdt_ping(struct watchdog_device *wdd)
return 0;
 }
 
-static unsigned int rti_wdt_get_timeleft(struct watchdog_device *wdd)
+static int rti_wdt_setup_hw_hb(struct watchdog_device *wdd, u32 wsize)
+{
+   /*
+* RTI only supports a windowed mode, where the watchdog can only
+* be petted during the open window; not too early or not too late.
+* The HW configuration options only allow for the open window size
+* to be 50% or less than that.
+*/
+   switch (wsize) {
+   case RTIWWDSIZE_50P:
+   /* 50% open window => 50% min heartbeat */
+   wdd->min_hw_heartbeat_ms = 500 * heartbeat;
+   break;
+
+   case RTIWWDSIZE_25P:
+   /* 25% open window => 75% min heartbeat */
+   wdd->min_hw_heartbeat_ms = 750 * heartbeat;
+   break;
+
+   case RTIWWDSIZE_12P5:
+   /* 12.5% open window => 87.5% min heartbeat */
+   wdd->min_hw_heartbeat_ms = 875 * heartbeat;
+   break;
+
+   case RTIWWDSIZE_6P25:
+   /* 6.5% open window => 93.5% min heartbeat */
+   wdd->min_hw_heartbeat_ms = 935 * heartbeat;
+   break;
+
+   case RTIWWDSIZE_3P125:
+   /* 3.125% open window => 96.9% min heartbeat */
+   wdd->min_hw_heartbeat_ms = 969 * heartbeat;
+   break;
+
+   default:
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
+static unsigned int rti_wdt_get_timeleft_ms(struct watchdog_device *wdd)
 {
u64 timer_counter;
u32 val;
@@ -123,11 +166,18 @@ static unsigned int rti_wdt_get_timeleft(struct 
watchdog_device *wdd)
 
timer_counter = readl_relaxed(wdt->base + RTIDWDCNTR);
 
+   timer_counter *= 1000;
+
do_div(timer_counter, wdt->freq);
 
return timer_counter;
 }
 
+static unsigned int rti_wdt_get_timeleft(struct watchdog_device *wdd)
+{
+   return rti_wdt_get_timeleft_ms(wdd) / 1000;
+}
+
 static const struct watchdog_info rti_wdt_info = {
.options = WDIOF_KEEPALIVEPING,
.identity = "K3 RTI Watchdog",
@@ -148,6 +198,7 @@ static int rti_wdt_probe(struct platform_device *pdev)
struct watchdog_device *wdd;
struct rti_wdt_device *wdt;
struct clk *clk;
+   u32 last_ping = 0;
 
wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
if (!wdt)
@@ -169,6 +220,14 @@ static int rti_wdt_probe(struct platform_device *pdev)
return -EINVAL;
}
 
+   /*
+* If watchdog is running at 32k clock, it is not accurate.
+* Adjust frequency down in this case so that we don't pet
+* the watchdog too often.
+*/
+   if (wdt->freq < 32768)
+   wdt->freq = wdt->freq * 9 / 10;
+
pm_runtime_enable(dev);
ret = pm_runtime_get_sync(dev);
if (ret) {
@@ -185,11 +244,8 @@ static int rti_wdt_probe(struct platform_device *pdev)
wdd->min_timeout =

[PATCHv3 4/4] watchdog: rti-wdt: balance pm runtime enable calls

2020-07-13 Thread Tero Kristo
PM runtime should be disabled in the fail path of probe and when
the driver is removed.

Fixes: 2d63908bdbfb ("watchdog: Add K3 RTI watchdog support")
Signed-off-by: Tero Kristo 
Reviewed-by: Guenter Roeck 
---
 drivers/watchdog/rti_wdt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
index 45dfc546e371..5cc1a1d654b6 100644
--- a/drivers/watchdog/rti_wdt.c
+++ b/drivers/watchdog/rti_wdt.c
@@ -302,6 +302,7 @@ static int rti_wdt_probe(struct platform_device *pdev)
 
 err_iomap:
pm_runtime_put_sync(>dev);
+   pm_runtime_disable(>dev);
 
return ret;
 }
@@ -312,6 +313,7 @@ static int rti_wdt_remove(struct platform_device *pdev)
 
watchdog_unregister_device(>wdd);
pm_runtime_put(>dev);
+   pm_runtime_disable(>dev);
 
return 0;
 }
-- 
2.17.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


[PATCHv3 1/4] watchdog: use __watchdog_ping in startup

2020-07-13 Thread Tero Kristo
Current watchdog startup functionality does not respect the minimum hw
heartbeat setup and the last watchdog ping timeframe when watchdog is
already running and userspace process attaches to it. Fix this by using
the __watchdog_ping from the startup also. For this code path, we can
also let the __watchdog_ping handle the bookkeeping for the worker and
last keepalive times.

Signed-off-by: Tero Kristo 
Reviewed-by: Guenter Roeck 
---
 drivers/watchdog/watchdog_dev.c | 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 7e4cd34a8c20..bc1cfa288553 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -275,15 +275,18 @@ static int watchdog_start(struct watchdog_device *wdd)
set_bit(_WDOG_KEEPALIVE, _data->status);
 
started_at = ktime_get();
-   if (watchdog_hw_running(wdd) && wdd->ops->ping)
-   err = wdd->ops->ping(wdd);
-   else
+   if (watchdog_hw_running(wdd) && wdd->ops->ping) {
+   err = __watchdog_ping(wdd);
+   if (err == 0)
+   set_bit(WDOG_ACTIVE, >status);
+   } else {
err = wdd->ops->start(wdd);
-   if (err == 0) {
-   set_bit(WDOG_ACTIVE, >status);
-   wd_data->last_keepalive = started_at;
-   wd_data->last_hw_keepalive = started_at;
-   watchdog_update_worker(wdd);
+   if (err == 0) {
+   set_bit(WDOG_ACTIVE, >status);
+   wd_data->last_keepalive = started_at;
+   wd_data->last_hw_keepalive = started_at;
+   watchdog_update_worker(wdd);
+   }
}
 
return err;
-- 
2.17.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCHv2 4/5] watchdog: rti-wdt: attach to running watchdog during probe

2020-07-13 Thread Tero Kristo

On 05/07/2020 18:07, Guenter Roeck wrote:

On 7/3/20 5:04 AM, Tero Kristo wrote:

If the RTI watchdog is running already during probe, the driver must
configure itself to match the HW. Window size and timeout is probed from
hardware, and the last keepalive ping is adjusted to match it also.

Signed-off-by: Tero Kristo 
---
  drivers/watchdog/rti_wdt.c | 26 +++---
  1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
index 110bfc8d0bb3..987e5a798cb4 100644
--- a/drivers/watchdog/rti_wdt.c
+++ b/drivers/watchdog/rti_wdt.c
@@ -213,6 +213,7 @@ static int rti_wdt_probe(struct platform_device *pdev)
struct watchdog_device *wdd;
struct rti_wdt_device *wdt;
struct clk *clk;
+   u32 last_ping = 0;
  
  	wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);

if (!wdt)
@@ -258,11 +259,8 @@ static int rti_wdt_probe(struct platform_device *pdev)
wdd->min_timeout = 1;
wdd->max_hw_heartbeat_ms = (WDT_PRELOAD_MAX << WDT_PRELOAD_SHIFT) /
wdt->freq * 1000;
-   wdd->timeout = DEFAULT_HEARTBEAT;


What if the watchdog is not running ?


Configuring wdd->timeout seems redundant, it gets set by the 
watchdog_init_timeout call done later. I just moved that post the check 
for a running watchdog so that the same call is used for both cases.





wdd->parent = dev;
  
-	watchdog_init_timeout(wdd, heartbeat, dev);

-
watchdog_set_drvdata(wdd, wdt);
watchdog_set_nowayout(wdd, 1);
watchdog_set_restart_priority(wdd, 128);
@@ -274,12 +272,34 @@ static int rti_wdt_probe(struct platform_device *pdev)
goto err_iomap;
}
  
+	if (readl(wdt->base + RTIDWDCTRL) == WDENABLE_KEY) {

+   u32 time_left;
+
+   set_bit(WDOG_HW_RUNNING, >status);
+   time_left = rti_wdt_get_timeleft(wdd);
+   heartbeat = readl(wdt->base + RTIDWDPRLD);
+   heartbeat <<= WDT_PRELOAD_SHIFT;
+   heartbeat /= wdt->freq;
+


This ignores any heartbeat configured as module parameter, which most
people will consider unexpected. It might be worthwhile documenting that.


I'll add a dev_warn for this case.




+   wsize = readl(wdt->base + RTIWWDSIZECTRL);
+   ret = rti_wdt_setup_hw_hb(wdd);
+   if (ret)
+   goto err_iomap;
+
+   last_ping = -(time_left - heartbeat) * 1000;


Why the double negation ?

last_ping = (heartbeat - time_left) * 1000;

seems simpler. Also, what if heartbeat - time_left is negative for whatever
reason ?


Will fix. I'll add a dev_warn for that case and assume last ping to be zero.



I am not sure if it is a good idea to call rti_wdt_get_timeleft()
here. It might be better to add a helper function such as
rti_wdt_get_timeleft_ms() to return the time left in milli-seconds
for improved accuracy.


Will add that.

-Tero




+   }
+
+   watchdog_init_timeout(wdd, heartbeat, dev);
+
ret = watchdog_register_device(wdd);
if (ret) {
dev_err(dev, "cannot register watchdog device\n");
goto err_iomap;
}
  
+	if (last_ping)

+   watchdog_set_last_hw_keepalive(wdd, last_ping);
+
return 0;
  
  err_iomap:






--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCHv2 3/5] watchdog: rti-wdt: add support for window size configuration

2020-07-13 Thread Tero Kristo

On 05/07/2020 17:49, Guenter Roeck wrote:

On 7/3/20 5:04 AM, Tero Kristo wrote:

RTI watchdog can support different open window sizes. Add support for
these and add a new module parameter to configure it. The default open
window size for the driver still remains at 50%.

Also, modify the margin calculation logic a bit for 32k source clock,
instead of adding a margin to every window config, assume the 32k source
clock is running slower.


I'll drop this patch mostly in next rev to get rid of the dynamic config 
for window size and always use 50%. I will just read the window size in 
case someone has started the watchdog beforehand.




Signed-off-by: Tero Kristo 
---
  drivers/watchdog/rti_wdt.c | 112 +++--
  1 file changed, 95 insertions(+), 17 deletions(-)

diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
index d456dd72d99a..110bfc8d0bb3 100644
--- a/drivers/watchdog/rti_wdt.c
+++ b/drivers/watchdog/rti_wdt.c
@@ -19,7 +19,8 @@
  #include 
  #include 
  
-#define DEFAULT_HEARTBEAT 60

+#define DEFAULT_HEARTBEAT  60
+#define DEFAULT_WINDOWSIZE 50
  
  /* Max heartbeat is calculated at 32kHz source clock */

  #define MAX_HEARTBEAT 1000
@@ -35,9 +36,13 @@
  
  #define RTIWWDRX_NMI	0xa
  
-#define RTIWWDSIZE_50P	0x50

+#define RTIWWDSIZE_50P 0x50
+#define RTIWWDSIZE_25P 0x500
+#define RTIWWDSIZE_12P50x5000
+#define RTIWWDSIZE_6P250x5
+#define RTIWWDSIZE_3P125   0x50
  
-#define WDENABLE_KEY	0xa98559da

+#define WDENABLE_KEY   0xa98559da
  
  #define WDKEY_SEQ0		0xe51a

  #define WDKEY_SEQ10xa35c
@@ -48,7 +53,8 @@
  
  #define DWDST			BIT(1)
  
-static int heartbeat;

+static int heartbeat = DEFAULT_HEARTBEAT;
+static u32 wsize = DEFAULT_WINDOWSIZE;
  
  /*

   * struct to hold data for each WDT device
@@ -62,34 +68,93 @@ struct rti_wdt_device {
struct watchdog_device  wdd;
  };
  
+static int rti_wdt_convert_wsize(void)

+{
+   if (wsize >= 50) {
+   wsize = RTIWWDSIZE_50P;
+   } else if (wsize >= 25) {
+   wsize = RTIWWDSIZE_25P;
+   } else if (wsize > 12) {
+   wsize = RTIWWDSIZE_12P5;
+   } else if (wsize > 6) {
+   wsize = RTIWWDSIZE_6P25;
+   } else if (wsize > 3) {
+   wsize = RTIWWDSIZE_3P125;
+   } else {
+   pr_err("%s: bad windowsize: %d\n", __func__, wsize);


Please do not use pr_ functions. Pass the watchdog device as argument
and use dev_err().

Also, this function modifies the wsize parameter. When called
again, that parameter will have a  totally different meaning, and
the second call to this function will always set the window size
to 50.

On top of all that, window sizes larger than 50 are set to 50,
window sizes between 4 and 49 are adjusted, and window sizes <= 3
are rejected. That is not exactly consistent.

Does this module parameter really add value / make sense ?
What is the use case ? We should not add such complexity without
use case.


I'll ditch the support for this. Just thought that it would be neat 
feature to have this in place because I ended up implementing most of 
this for the attach feature anyways.





+   return -EINVAL;
+   }
+
+   return 0;
+}
+
+static int rti_wdt_setup_hw_hb(struct watchdog_device *wdd)
+{
+   /*
+* RTI only supports a windowed mode, where the watchdog can only
+* be petted during the open window; not too early or not too late.
+* The HW configuration options only allow for the open window size
+* to be 50% or less than that.
+*/
+   switch (wsize) {
+   case RTIWWDSIZE_50P:
+   /* 50% open window => 50% min heartbeat */
+   wdd->min_hw_heartbeat_ms = 500 * heartbeat;
+   break;
+
+   case RTIWWDSIZE_25P:
+   /* 25% open window => 75% min heartbeat */
+   wdd->min_hw_heartbeat_ms = 750 * heartbeat;
+   break;
+
+   case RTIWWDSIZE_12P5:
+   /* 12.5% open window => 87.5% min heartbeat */
+   wdd->min_hw_heartbeat_ms = 875 * heartbeat;
+   break;
+
+   case RTIWWDSIZE_6P25:
+   /* 6.5% open window => 93.5% min heartbeat */
+   wdd->min_hw_heartbeat_ms = 935 * heartbeat;
+   break;
+
+   case RTIWWDSIZE_3P125:
+   /* 3.125% open window => 96.9% min heartbeat */
+   wdd->min_hw_heartbeat_ms = 969 * heartbeat;
+   break;
+
+   default:
+   pr_err("%s: Bad watchdog window size!\n", __func__);


Same here.


+   return -EINVAL;
+   }
+
+   return 0;
+}
+
  static int rti_wdt_start(struct watchdog_device *wdd)
  {
u32 timer_margin;
struct rti_wdt_device *wdt = watchdog_get_drvdata(wdd);
+   int ret;
  
  	/* s

Re: [PATCHv2 2/5] watchdog: add support for adjusting last known HW keepalive time

2020-07-13 Thread Tero Kristo

On 05/07/2020 17:58, Guenter Roeck wrote:

On 7/3/20 5:04 AM, Tero Kristo wrote:

Certain watchdogs require the watchdog only to be pinged within a
specific time window, pinging too early or too late cause the watchdog
to fire. In cases where this sort of watchdog has been started before
kernel comes up, we must adjust the watchdog keepalive window to match
the actually running timer, so add a new driver API for this purpose.

Signed-off-by: Tero Kristo 
---
  drivers/watchdog/watchdog_dev.c | 23 +++
  include/linux/watchdog.h|  2 ++
  2 files changed, 25 insertions(+)

diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index bc1cfa288553..5848551cf29d 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -1138,6 +1138,29 @@ void watchdog_dev_unregister(struct watchdog_device *wdd)
watchdog_cdev_unregister(wdd);
  }
  
+/*

+ * watchdog_set_last_hw_keepalive: set last HW keepalive time for watchdog
+ *
+ * Adjusts the last known HW keepalive time for a watchdog timer.
+ * This is needed in case where watchdog has been started before
+ * kernel by someone like bootloader, and it can't be pinged


... needed if the watchdog is already running when the probe function
is called, and ...


+ * immediately. This adjusts the watchdog ping period to match
+ * the currently running timer.


It doesn't adjust the ping period.


+ */


last_ping_ms needs to be documented (the last heartbeat was last_ping_ms
milliseconds ago ?), both here and in 
Documentation/watchdog/watchdog-kernel-api.rst.
It needs to be documented that the function must be called immediately
after watchdog registration, and that min_hw_heartbeat_ms must
be set for it to be useful.


+int watchdog_set_last_hw_keepalive(struct watchdog_device *wdd,
+  unsigned int last_ping_ms)
+{
+   struct watchdog_core_data *wd_data = wdd->wd_data;


This needs a NULL check, in case it is called before watchdog driver
registration.


Ok will fix all the above in next revision.

-Tero




+   ktime_t now;
+
+   now = ktime_get();
+
+   wd_data->last_hw_keepalive = ktime_sub(now, ms_to_ktime(last_ping_ms));
+
+   return __watchdog_ping(wdd);
+}
+EXPORT_SYMBOL_GPL(watchdog_set_last_hw_keepalive);
+
  /*
   *watchdog_dev_init: init dev part of watchdog core
   *
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 1464ce6ffa31..9b19e6bb68b5 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -210,6 +210,8 @@ extern int watchdog_init_timeout(struct watchdog_device 
*wdd,
  extern int watchdog_register_device(struct watchdog_device *);
  extern void watchdog_unregister_device(struct watchdog_device *);
  
+int watchdog_set_last_hw_keepalive(struct watchdog_device *, unsigned int);

+
  /* devres register variant */
  int devm_watchdog_register_device(struct device *dev, struct watchdog_device 
*);
  





--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH] EDAC-TI: Replace HTTP links with HTTPS ones

2020-07-13 Thread Tero Kristo

On 09/07/2020 20:47, Alexander A. Klimov wrote:

Rationale:
Reduces attack surface on kernel devs opening the links for MITM
as HTTPS traffic is much harder to manipulate.

Deterministic algorithm:
For each file:
   If not .svg:
 For each line:
   If doesn't contain `\bxmlns\b`:
 For each link, `\bhttp://[^# \t\r\n]*(?:\w|/)`:
  If neither `\bgnu\.org/license`, nor `\bmozilla\.org/MPL\b`:
 If both the HTTP and HTTPS versions
 return 200 OK and serve the same content:
   Replace HTTP with HTTPS.

Signed-off-by: Alexander A. Klimov 


Looks fine to me.

Acked-by: Tero Kristo 


---
  Continuing my work started at 93431e0607e5.
  See also: git log --oneline '--author=Alexander A. Klimov 
' v5.7..master
  (Actually letting a shell for loop submit all this stuff for me.)

  If there are any URLs to be removed completely or at least not HTTPSified:
  Just clearly say so and I'll *undo my change*.
  See also: https://lkml.org/lkml/2020/6/27/64

  If there are any valid, but yet not changed URLs:
  See: https://lkml.org/lkml/2020/6/26/837

  If you apply the patch, please let me know.


  drivers/edac/ti_edac.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/edac/ti_edac.c b/drivers/edac/ti_edac.c
index 8be3e89a510e..6e52796a0b41 100644
--- a/drivers/edac/ti_edac.c
+++ b/drivers/edac/ti_edac.c
@@ -1,6 +1,6 @@
  // SPDX-License-Identifier: GPL-2.0
  /*
- * Copyright (C) 2017 Texas Instruments Incorporated - http://www.ti.com/
+ * Copyright (C) 2017 Texas Instruments Incorporated - https://www.ti.com/
   *
   * Texas Instruments DDR3 ECC error correction and detection driver
   *



--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


[PATCHv2 3/5] watchdog: rti-wdt: add support for window size configuration

2020-07-03 Thread Tero Kristo
RTI watchdog can support different open window sizes. Add support for
these and add a new module parameter to configure it. The default open
window size for the driver still remains at 50%.

Also, modify the margin calculation logic a bit for 32k source clock,
instead of adding a margin to every window config, assume the 32k source
clock is running slower.

Signed-off-by: Tero Kristo 
---
 drivers/watchdog/rti_wdt.c | 112 +++--
 1 file changed, 95 insertions(+), 17 deletions(-)

diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
index d456dd72d99a..110bfc8d0bb3 100644
--- a/drivers/watchdog/rti_wdt.c
+++ b/drivers/watchdog/rti_wdt.c
@@ -19,7 +19,8 @@
 #include 
 #include 
 
-#define DEFAULT_HEARTBEAT 60
+#define DEFAULT_HEARTBEAT  60
+#define DEFAULT_WINDOWSIZE 50
 
 /* Max heartbeat is calculated at 32kHz source clock */
 #define MAX_HEARTBEAT  1000
@@ -35,9 +36,13 @@
 
 #define RTIWWDRX_NMI   0xa
 
-#define RTIWWDSIZE_50P 0x50
+#define RTIWWDSIZE_50P 0x50
+#define RTIWWDSIZE_25P 0x500
+#define RTIWWDSIZE_12P50x5000
+#define RTIWWDSIZE_6P250x5
+#define RTIWWDSIZE_3P125   0x50
 
-#define WDENABLE_KEY   0xa98559da
+#define WDENABLE_KEY   0xa98559da
 
 #define WDKEY_SEQ0 0xe51a
 #define WDKEY_SEQ1 0xa35c
@@ -48,7 +53,8 @@
 
 #define DWDST  BIT(1)
 
-static int heartbeat;
+static int heartbeat = DEFAULT_HEARTBEAT;
+static u32 wsize = DEFAULT_WINDOWSIZE;
 
 /*
  * struct to hold data for each WDT device
@@ -62,34 +68,93 @@ struct rti_wdt_device {
struct watchdog_device  wdd;
 };
 
+static int rti_wdt_convert_wsize(void)
+{
+   if (wsize >= 50) {
+   wsize = RTIWWDSIZE_50P;
+   } else if (wsize >= 25) {
+   wsize = RTIWWDSIZE_25P;
+   } else if (wsize > 12) {
+   wsize = RTIWWDSIZE_12P5;
+   } else if (wsize > 6) {
+   wsize = RTIWWDSIZE_6P25;
+   } else if (wsize > 3) {
+   wsize = RTIWWDSIZE_3P125;
+   } else {
+   pr_err("%s: bad windowsize: %d\n", __func__, wsize);
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
+static int rti_wdt_setup_hw_hb(struct watchdog_device *wdd)
+{
+   /*
+* RTI only supports a windowed mode, where the watchdog can only
+* be petted during the open window; not too early or not too late.
+* The HW configuration options only allow for the open window size
+* to be 50% or less than that.
+*/
+   switch (wsize) {
+   case RTIWWDSIZE_50P:
+   /* 50% open window => 50% min heartbeat */
+   wdd->min_hw_heartbeat_ms = 500 * heartbeat;
+   break;
+
+   case RTIWWDSIZE_25P:
+   /* 25% open window => 75% min heartbeat */
+   wdd->min_hw_heartbeat_ms = 750 * heartbeat;
+   break;
+
+   case RTIWWDSIZE_12P5:
+   /* 12.5% open window => 87.5% min heartbeat */
+   wdd->min_hw_heartbeat_ms = 875 * heartbeat;
+   break;
+
+   case RTIWWDSIZE_6P25:
+   /* 6.5% open window => 93.5% min heartbeat */
+   wdd->min_hw_heartbeat_ms = 935 * heartbeat;
+   break;
+
+   case RTIWWDSIZE_3P125:
+   /* 3.125% open window => 96.9% min heartbeat */
+   wdd->min_hw_heartbeat_ms = 969 * heartbeat;
+   break;
+
+   default:
+   pr_err("%s: Bad watchdog window size!\n", __func__);
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
 static int rti_wdt_start(struct watchdog_device *wdd)
 {
u32 timer_margin;
struct rti_wdt_device *wdt = watchdog_get_drvdata(wdd);
+   int ret;
 
/* set timeout period */
-   timer_margin = (u64)wdd->timeout * wdt->freq;
+   timer_margin = (u64)heartbeat * wdt->freq;
timer_margin >>= WDT_PRELOAD_SHIFT;
if (timer_margin > WDT_PRELOAD_MAX)
timer_margin = WDT_PRELOAD_MAX;
writel_relaxed(timer_margin, wdt->base + RTIDWDPRLD);
 
-   /*
-* RTI only supports a windowed mode, where the watchdog can only
-* be petted during the open window; not too early or not too late.
-* The HW configuration options only allow for the open window size
-* to be 50% or less than that; we obviouly want to configure the open
-* window as large as possible so we select the 50% option. To avoid
-* any glitches, we accommodate 5% safety margin also, so we setup
-* the min_hw_hearbeat at 55% of the timeout period.
-*/
-   wdd->min_hw_heartbeat_ms = 11 * wdd->timeout * 1000 / 20;
+   ret = rti_wdt_convert_wsize();
+   if (ret)
+   return ret;
+
+   ret = rti_wdt_setup_hw_hb(w

[PATCHv2] watchdog: rti-wdt: support attaching to running wdt

2020-07-03 Thread Tero Kristo
Hi,

Version 2 of the series has quite a few changes based on feedback on
previous version.

1) Add new watchdog core API for adjusting the last hw keepalive time
   (Patch #2)
2) Modify the driver to support adjusting the window size, current
   driver only supports 50% window size. The window size is configured
   via a module parameter.
3) Modify the attach mechanism to running wdt to configure the heartbeat
   and window size based on running HW wdt setup.
4) Fix a runtime PM issue that was noticed while dealing with the new
   module parameter (tested via rmmod / modprobe)

-Tero


--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


[PATCHv2 2/5] watchdog: add support for adjusting last known HW keepalive time

2020-07-03 Thread Tero Kristo
Certain watchdogs require the watchdog only to be pinged within a
specific time window, pinging too early or too late cause the watchdog
to fire. In cases where this sort of watchdog has been started before
kernel comes up, we must adjust the watchdog keepalive window to match
the actually running timer, so add a new driver API for this purpose.

Signed-off-by: Tero Kristo 
---
 drivers/watchdog/watchdog_dev.c | 23 +++
 include/linux/watchdog.h|  2 ++
 2 files changed, 25 insertions(+)

diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index bc1cfa288553..5848551cf29d 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -1138,6 +1138,29 @@ void watchdog_dev_unregister(struct watchdog_device *wdd)
watchdog_cdev_unregister(wdd);
 }
 
+/*
+ * watchdog_set_last_hw_keepalive: set last HW keepalive time for watchdog
+ *
+ * Adjusts the last known HW keepalive time for a watchdog timer.
+ * This is needed in case where watchdog has been started before
+ * kernel by someone like bootloader, and it can't be pinged
+ * immediately. This adjusts the watchdog ping period to match
+ * the currently running timer.
+ */
+int watchdog_set_last_hw_keepalive(struct watchdog_device *wdd,
+  unsigned int last_ping_ms)
+{
+   struct watchdog_core_data *wd_data = wdd->wd_data;
+   ktime_t now;
+
+   now = ktime_get();
+
+   wd_data->last_hw_keepalive = ktime_sub(now, ms_to_ktime(last_ping_ms));
+
+   return __watchdog_ping(wdd);
+}
+EXPORT_SYMBOL_GPL(watchdog_set_last_hw_keepalive);
+
 /*
  * watchdog_dev_init: init dev part of watchdog core
  *
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 1464ce6ffa31..9b19e6bb68b5 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -210,6 +210,8 @@ extern int watchdog_init_timeout(struct watchdog_device 
*wdd,
 extern int watchdog_register_device(struct watchdog_device *);
 extern void watchdog_unregister_device(struct watchdog_device *);
 
+int watchdog_set_last_hw_keepalive(struct watchdog_device *, unsigned int);
+
 /* devres register variant */
 int devm_watchdog_register_device(struct device *dev, struct watchdog_device 
*);
 
-- 
2.17.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


[PATCHv2 4/5] watchdog: rti-wdt: attach to running watchdog during probe

2020-07-03 Thread Tero Kristo
If the RTI watchdog is running already during probe, the driver must
configure itself to match the HW. Window size and timeout is probed from
hardware, and the last keepalive ping is adjusted to match it also.

Signed-off-by: Tero Kristo 
---
 drivers/watchdog/rti_wdt.c | 26 +++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
index 110bfc8d0bb3..987e5a798cb4 100644
--- a/drivers/watchdog/rti_wdt.c
+++ b/drivers/watchdog/rti_wdt.c
@@ -213,6 +213,7 @@ static int rti_wdt_probe(struct platform_device *pdev)
struct watchdog_device *wdd;
struct rti_wdt_device *wdt;
struct clk *clk;
+   u32 last_ping = 0;
 
wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
if (!wdt)
@@ -258,11 +259,8 @@ static int rti_wdt_probe(struct platform_device *pdev)
wdd->min_timeout = 1;
wdd->max_hw_heartbeat_ms = (WDT_PRELOAD_MAX << WDT_PRELOAD_SHIFT) /
wdt->freq * 1000;
-   wdd->timeout = DEFAULT_HEARTBEAT;
wdd->parent = dev;
 
-   watchdog_init_timeout(wdd, heartbeat, dev);
-
watchdog_set_drvdata(wdd, wdt);
watchdog_set_nowayout(wdd, 1);
watchdog_set_restart_priority(wdd, 128);
@@ -274,12 +272,34 @@ static int rti_wdt_probe(struct platform_device *pdev)
goto err_iomap;
}
 
+   if (readl(wdt->base + RTIDWDCTRL) == WDENABLE_KEY) {
+   u32 time_left;
+
+   set_bit(WDOG_HW_RUNNING, >status);
+   time_left = rti_wdt_get_timeleft(wdd);
+   heartbeat = readl(wdt->base + RTIDWDPRLD);
+   heartbeat <<= WDT_PRELOAD_SHIFT;
+   heartbeat /= wdt->freq;
+
+   wsize = readl(wdt->base + RTIWWDSIZECTRL);
+   ret = rti_wdt_setup_hw_hb(wdd);
+   if (ret)
+   goto err_iomap;
+
+   last_ping = -(time_left - heartbeat) * 1000;
+   }
+
+   watchdog_init_timeout(wdd, heartbeat, dev);
+
ret = watchdog_register_device(wdd);
if (ret) {
dev_err(dev, "cannot register watchdog device\n");
goto err_iomap;
}
 
+   if (last_ping)
+   watchdog_set_last_hw_keepalive(wdd, last_ping);
+
return 0;
 
 err_iomap:
-- 
2.17.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


[PATCHv2 5/5] watchdog: rti-wdt: balance pm runtime enable calls

2020-07-03 Thread Tero Kristo
PM runtime should be disabled in the fail path of probe and when
the driver is removed.

Fixes: 2d63908bdbfb ("watchdog: Add K3 RTI watchdog support")
Signed-off-by: Tero Kristo 
---
 drivers/watchdog/rti_wdt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
index 987e5a798cb4..7007445da80b 100644
--- a/drivers/watchdog/rti_wdt.c
+++ b/drivers/watchdog/rti_wdt.c
@@ -304,6 +304,7 @@ static int rti_wdt_probe(struct platform_device *pdev)
 
 err_iomap:
pm_runtime_put_sync(>dev);
+   pm_runtime_disable(>dev);
 
return ret;
 }
@@ -314,6 +315,7 @@ static int rti_wdt_remove(struct platform_device *pdev)
 
watchdog_unregister_device(>wdd);
pm_runtime_put(>dev);
+   pm_runtime_disable(>dev);
 
return 0;
 }
-- 
2.17.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


[PATCHv2 1/5] watchdog: use __watchdog_ping in startup

2020-07-03 Thread Tero Kristo
Current watchdog startup functionality does not respect the minimum hw
heartbeat setup and the last watchdog ping timeframe when watchdog is
already running and userspace process attaches to it. Fix this by using
the __watchdog_ping from the startup also. For this code path, we can
also let the __watchdog_ping handle the bookkeeping for the worker and
last keepalive times.

Signed-off-by: Tero Kristo 
Reviewed-by: Guenter Roeck 
---
 drivers/watchdog/watchdog_dev.c | 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 7e4cd34a8c20..bc1cfa288553 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -275,15 +275,18 @@ static int watchdog_start(struct watchdog_device *wdd)
set_bit(_WDOG_KEEPALIVE, _data->status);
 
started_at = ktime_get();
-   if (watchdog_hw_running(wdd) && wdd->ops->ping)
-   err = wdd->ops->ping(wdd);
-   else
+   if (watchdog_hw_running(wdd) && wdd->ops->ping) {
+   err = __watchdog_ping(wdd);
+   if (err == 0)
+   set_bit(WDOG_ACTIVE, >status);
+   } else {
err = wdd->ops->start(wdd);
-   if (err == 0) {
-   set_bit(WDOG_ACTIVE, >status);
-   wd_data->last_keepalive = started_at;
-   wd_data->last_hw_keepalive = started_at;
-   watchdog_update_worker(wdd);
+   if (err == 0) {
+   set_bit(WDOG_ACTIVE, >status);
+   wd_data->last_keepalive = started_at;
+   wd_data->last_hw_keepalive = started_at;
+   watchdog_update_worker(wdd);
+   }
}
 
return err;
-- 
2.17.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH 2/2] watchdog: rti: tweak min_hw_heartbeat_ms to match initial allowed window

2020-07-01 Thread Tero Kristo

On 01/07/2020 16:34, Guenter Roeck wrote:

On 6/30/20 10:50 PM, Tero Kristo wrote:
[ ... ]


Hardware supports changing the timeout value, however it only updates this 
during the next window (preload values are picked once user pings the watchdog.)


The current driver doesn't support or use that, though, since the start
function is only called once to start the watchdog, and not at all if
the watchdog is already running. So, if the bootloader sets the timeout
to X, and the user sets a timeout of, say, X * 4, userspace will never
ping the watchdog often enough. The driver will have to address that
to support already-running watchdogs.


Yeah, I will modify that. I think I will just prevent changing the 
timeout if watchdog has been enabled from boot, it is probably cleanest 
approach. Unless I happen to come up with some sane way to change it on fly.


-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH 2/2] watchdog: rti: tweak min_hw_heartbeat_ms to match initial allowed window

2020-06-30 Thread Tero Kristo

On 30/06/2020 23:23, Guenter Roeck wrote:

On Thu, Jun 25, 2020 at 08:04:50PM +0300, Tero Kristo wrote:

On 25/06/2020 16:35, Guenter Roeck wrote:

On 6/25/20 1:32 AM, Tero Kristo wrote:

On 24/06/2020 18:24, Jan Kiszka wrote:

On 24.06.20 13:45, Tero Kristo wrote:

If the RTI watchdog has been started by someone (like bootloader) when
the driver probes, we must adjust the initial ping timeout to match the
currently running watchdog window to avoid generating watchdog reset.

Signed-off-by: Tero Kristo 
---
    drivers/watchdog/rti_wdt.c | 25 +
    1 file changed, 25 insertions(+)

diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
index d456dd72d99a..02ea2b2435f5 100644
--- a/drivers/watchdog/rti_wdt.c
+++ b/drivers/watchdog/rti_wdt.c
@@ -55,11 +55,13 @@ static int heartbeat;
     * @base - base io address of WD device
     * @freq - source clock frequency of WDT
     * @wdd  - hold watchdog device as is in WDT core
+ * @min_hw_heartbeat_save - save of the min hw heartbeat value
     */
    struct rti_wdt_device {
    void __iomem    *base;
    unsigned long    freq;
    struct watchdog_device    wdd;
+    unsigned int    min_hw_heartbeat_save;
    };
    static int rti_wdt_start(struct watchdog_device *wdd)
@@ -107,6 +109,11 @@ static int rti_wdt_ping(struct watchdog_device *wdd)
    /* put watchdog in active state */
    writel_relaxed(WDKEY_SEQ1, wdt->base + RTIWDKEY);
+    if (wdt->min_hw_heartbeat_save) {
+    wdd->min_hw_heartbeat_ms = wdt->min_hw_heartbeat_save;
+    wdt->min_hw_heartbeat_save = 0;
+    }
+
    return 0;
    }
@@ -201,6 +208,24 @@ static int rti_wdt_probe(struct platform_device *pdev)
    goto err_iomap;
    }
+    if (readl(wdt->base + RTIDWDCTRL) == WDENABLE_KEY) {
+    u32 time_left;
+    u32 heartbeat;
+
+    set_bit(WDOG_HW_RUNNING, >status);
+    time_left = rti_wdt_get_timeleft(wdd);
+    heartbeat = readl(wdt->base + RTIDWDPRLD);
+    heartbeat <<= WDT_PRELOAD_SHIFT;
+    heartbeat /= wdt->freq;
+    if (time_left < heartbeat / 2)
+    wdd->min_hw_heartbeat_ms = 0;
+    else
+    wdd->min_hw_heartbeat_ms =
+    (time_left - heartbeat / 2 + 1) * 1000;
+
+    wdt->min_hw_heartbeat_save = 11 * heartbeat * 1000 / 20;
+    }
+
    ret = watchdog_register_device(wdd);
    if (ret) {
    dev_err(dev, "cannot register watchdog device\n");



This assumes that the bootloader also programmed a 50% window, right? The 
pending U-Boot patch will do that, but what if that may chance or someone uses 
a different setup?


Yes, we assume 50%. I think based on the hw design, 50% is the only sane value 
to be used, otherwise you just shrink the open window too much and for no 
apparent reason.



Not sure if that is a valid assumption. Someone who designs a watchdog
with such a narrow ping window might as well also use it. The question
is if you want to rely on that assumption, or check and change it if needed.


Right, if that is a blocker, I can modify the code. Should be maybe couple
of lines addition.


Also, I wonder if we should add an API function such as
"set_last_hw_keepalive()" to avoid all that complexity.


I can try adding that also if it is desirable.



But wait, the code doesn't really match what the description of this
patch claims, or at least the description is misleading. Per the
description, this is to prevent an early timeout. However, the problem
here is that the watchdog core does not generate a ping, even if
requested, because it believes that it just generated one right before
the watchdog timer was registered, and that it can not generate another
one because min_hw_heartbeat_ms has not elapsed.


You are right. Maybe the patch description could use some more beef into it.



With that in mind, the problem is a bit more complex.

First, the driver doesn't really update the current timeout to the
value that is currently configured and enabled. Instead, it just
uses/assumes the default (DEFAULT_HEARTBEAT or whatever the heartbeat
module parameter is set to). This means that it is still possible for
an early timeout to occur if there is a mismatch between the bootloader
timeout and the timeout assumed by the driver. Worse, the timeout
is only updated in the start function - and the start function isn't
called if the watchdog is already running. Actually, the driver does
not support updating the timeout at all. This means that a mismatch
between the bootloader timeout and the timeout assumed by the driver
is not handled well.

To solve this, the driver would have to update the actual timeout to
whatever is programmed into the chip and ignore any module parameter
and default settings if the watchdog is already running. Alternatively,
it would have to support updating the timeout (if the hardware supports
that) after the wat

Re: [PATCH 2/2] watchdog: rti: tweak min_hw_heartbeat_ms to match initial allowed window

2020-06-25 Thread Tero Kristo

On 25/06/2020 16:35, Guenter Roeck wrote:

On 6/25/20 1:32 AM, Tero Kristo wrote:

On 24/06/2020 18:24, Jan Kiszka wrote:

On 24.06.20 13:45, Tero Kristo wrote:

If the RTI watchdog has been started by someone (like bootloader) when
the driver probes, we must adjust the initial ping timeout to match the
currently running watchdog window to avoid generating watchdog reset.

Signed-off-by: Tero Kristo 
---
   drivers/watchdog/rti_wdt.c | 25 +
   1 file changed, 25 insertions(+)

diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
index d456dd72d99a..02ea2b2435f5 100644
--- a/drivers/watchdog/rti_wdt.c
+++ b/drivers/watchdog/rti_wdt.c
@@ -55,11 +55,13 @@ static int heartbeat;
    * @base - base io address of WD device
    * @freq - source clock frequency of WDT
    * @wdd  - hold watchdog device as is in WDT core
+ * @min_hw_heartbeat_save - save of the min hw heartbeat value
    */
   struct rti_wdt_device {
   void __iomem    *base;
   unsigned long    freq;
   struct watchdog_device    wdd;
+    unsigned int    min_hw_heartbeat_save;
   };
   static int rti_wdt_start(struct watchdog_device *wdd)
@@ -107,6 +109,11 @@ static int rti_wdt_ping(struct watchdog_device *wdd)
   /* put watchdog in active state */
   writel_relaxed(WDKEY_SEQ1, wdt->base + RTIWDKEY);
+    if (wdt->min_hw_heartbeat_save) {
+    wdd->min_hw_heartbeat_ms = wdt->min_hw_heartbeat_save;
+    wdt->min_hw_heartbeat_save = 0;
+    }
+
   return 0;
   }
@@ -201,6 +208,24 @@ static int rti_wdt_probe(struct platform_device *pdev)
   goto err_iomap;
   }
+    if (readl(wdt->base + RTIDWDCTRL) == WDENABLE_KEY) {
+    u32 time_left;
+    u32 heartbeat;
+
+    set_bit(WDOG_HW_RUNNING, >status);
+    time_left = rti_wdt_get_timeleft(wdd);
+    heartbeat = readl(wdt->base + RTIDWDPRLD);
+    heartbeat <<= WDT_PRELOAD_SHIFT;
+    heartbeat /= wdt->freq;
+    if (time_left < heartbeat / 2)
+    wdd->min_hw_heartbeat_ms = 0;
+    else
+    wdd->min_hw_heartbeat_ms =
+    (time_left - heartbeat / 2 + 1) * 1000;
+
+    wdt->min_hw_heartbeat_save = 11 * heartbeat * 1000 / 20;
+    }
+
   ret = watchdog_register_device(wdd);
   if (ret) {
   dev_err(dev, "cannot register watchdog device\n");



This assumes that the bootloader also programmed a 50% window, right? The 
pending U-Boot patch will do that, but what if that may chance or someone uses 
a different setup?


Yes, we assume 50%. I think based on the hw design, 50% is the only sane value 
to be used, otherwise you just shrink the open window too much and for no 
apparent reason.



Not sure if that is a valid assumption. Someone who designs a watchdog
with such a narrow ping window might as well also use it. The question
is if you want to rely on that assumption, or check and change it if needed.


Right, if that is a blocker, I can modify the code. Should be maybe 
couple of lines addition.



Also, I wonder if we should add an API function such as
"set_last_hw_keepalive()" to avoid all that complexity.


I can try adding that also if it is desirable.

-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH 2/2] watchdog: rti: tweak min_hw_heartbeat_ms to match initial allowed window

2020-06-25 Thread Tero Kristo

On 24/06/2020 18:24, Jan Kiszka wrote:

On 24.06.20 13:45, Tero Kristo wrote:

If the RTI watchdog has been started by someone (like bootloader) when
the driver probes, we must adjust the initial ping timeout to match the
currently running watchdog window to avoid generating watchdog reset.

Signed-off-by: Tero Kristo 
---
  drivers/watchdog/rti_wdt.c | 25 +
  1 file changed, 25 insertions(+)

diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
index d456dd72d99a..02ea2b2435f5 100644
--- a/drivers/watchdog/rti_wdt.c
+++ b/drivers/watchdog/rti_wdt.c
@@ -55,11 +55,13 @@ static int heartbeat;
   * @base - base io address of WD device
   * @freq - source clock frequency of WDT
   * @wdd  - hold watchdog device as is in WDT core
+ * @min_hw_heartbeat_save - save of the min hw heartbeat value
   */
  struct rti_wdt_device {
  void __iomem    *base;
  unsigned long    freq;
  struct watchdog_device    wdd;
+    unsigned int    min_hw_heartbeat_save;
  };
  static int rti_wdt_start(struct watchdog_device *wdd)
@@ -107,6 +109,11 @@ static int rti_wdt_ping(struct watchdog_device *wdd)
  /* put watchdog in active state */
  writel_relaxed(WDKEY_SEQ1, wdt->base + RTIWDKEY);
+    if (wdt->min_hw_heartbeat_save) {
+    wdd->min_hw_heartbeat_ms = wdt->min_hw_heartbeat_save;
+    wdt->min_hw_heartbeat_save = 0;
+    }
+
  return 0;
  }
@@ -201,6 +208,24 @@ static int rti_wdt_probe(struct platform_device 
*pdev)

  goto err_iomap;
  }
+    if (readl(wdt->base + RTIDWDCTRL) == WDENABLE_KEY) {
+    u32 time_left;
+    u32 heartbeat;
+
+    set_bit(WDOG_HW_RUNNING, >status);
+    time_left = rti_wdt_get_timeleft(wdd);
+    heartbeat = readl(wdt->base + RTIDWDPRLD);
+    heartbeat <<= WDT_PRELOAD_SHIFT;
+    heartbeat /= wdt->freq;
+    if (time_left < heartbeat / 2)
+    wdd->min_hw_heartbeat_ms = 0;
+    else
+    wdd->min_hw_heartbeat_ms =
+    (time_left - heartbeat / 2 + 1) * 1000;
+
+    wdt->min_hw_heartbeat_save = 11 * heartbeat * 1000 / 20;
+    }
+
  ret = watchdog_register_device(wdd);
  if (ret) {
  dev_err(dev, "cannot register watchdog device\n");



This assumes that the bootloader also programmed a 50% window, right? 
The pending U-Boot patch will do that, but what if that may chance or 
someone uses a different setup?


Yes, we assume 50%. I think based on the hw design, 50% is the only sane 
value to be used, otherwise you just shrink the open window too much and 
for no apparent reason.


-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


[PATCH 0/2] watchdog: rti: adjust initial ping for attach

2020-06-24 Thread Tero Kristo
Hi,

This series fixes attaching the RTI watchdog driver to an already
running timer; it can be started by boot loader for example. In this
case, we must read the current remaining timeout, and adjust the
min_hw_heartbeat based on that so that we don't attempt to either pet
the watchdog too early, or too late.

The reason for all this is because the K3 RTI watchdog runs in windowed
mode only, petting it either too early, or too late causes a watchdog
reset.

-Tero


--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


[PATCH 2/2] watchdog: rti: tweak min_hw_heartbeat_ms to match initial allowed window

2020-06-24 Thread Tero Kristo
If the RTI watchdog has been started by someone (like bootloader) when
the driver probes, we must adjust the initial ping timeout to match the
currently running watchdog window to avoid generating watchdog reset.

Signed-off-by: Tero Kristo 
---
 drivers/watchdog/rti_wdt.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
index d456dd72d99a..02ea2b2435f5 100644
--- a/drivers/watchdog/rti_wdt.c
+++ b/drivers/watchdog/rti_wdt.c
@@ -55,11 +55,13 @@ static int heartbeat;
  * @base - base io address of WD device
  * @freq - source clock frequency of WDT
  * @wdd  - hold watchdog device as is in WDT core
+ * @min_hw_heartbeat_save - save of the min hw heartbeat value
  */
 struct rti_wdt_device {
void __iomem*base;
unsigned long   freq;
struct watchdog_device  wdd;
+   unsigned intmin_hw_heartbeat_save;
 };
 
 static int rti_wdt_start(struct watchdog_device *wdd)
@@ -107,6 +109,11 @@ static int rti_wdt_ping(struct watchdog_device *wdd)
/* put watchdog in active state */
writel_relaxed(WDKEY_SEQ1, wdt->base + RTIWDKEY);
 
+   if (wdt->min_hw_heartbeat_save) {
+   wdd->min_hw_heartbeat_ms = wdt->min_hw_heartbeat_save;
+   wdt->min_hw_heartbeat_save = 0;
+   }
+
return 0;
 }
 
@@ -201,6 +208,24 @@ static int rti_wdt_probe(struct platform_device *pdev)
goto err_iomap;
}
 
+   if (readl(wdt->base + RTIDWDCTRL) == WDENABLE_KEY) {
+   u32 time_left;
+   u32 heartbeat;
+
+   set_bit(WDOG_HW_RUNNING, >status);
+   time_left = rti_wdt_get_timeleft(wdd);
+   heartbeat = readl(wdt->base + RTIDWDPRLD);
+   heartbeat <<= WDT_PRELOAD_SHIFT;
+   heartbeat /= wdt->freq;
+   if (time_left < heartbeat / 2)
+   wdd->min_hw_heartbeat_ms = 0;
+   else
+   wdd->min_hw_heartbeat_ms =
+   (time_left - heartbeat / 2 + 1) * 1000;
+
+   wdt->min_hw_heartbeat_save = 11 * heartbeat * 1000 / 20;
+   }
+
ret = watchdog_register_device(wdd);
if (ret) {
dev_err(dev, "cannot register watchdog device\n");
-- 
2.17.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


[PATCH 1/2] watchdog: use __watchdog_ping in startup

2020-06-24 Thread Tero Kristo
Current watchdog startup functionality does not respect the minimum hw
heartbeat setup and the last watchdog ping timeframe when watchdog is
already running and userspace process attaches to it. Fix this by using
the __watchdog_ping from the startup also. For this code path, we can
also let the __watchdog_ping handle the bookkeeping for the worker and
last keepalive times.

Signed-off-by: Tero Kristo 
---
 drivers/watchdog/watchdog_dev.c | 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 7e4cd34a8c20..bc1cfa288553 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -275,15 +275,18 @@ static int watchdog_start(struct watchdog_device *wdd)
set_bit(_WDOG_KEEPALIVE, _data->status);
 
started_at = ktime_get();
-   if (watchdog_hw_running(wdd) && wdd->ops->ping)
-   err = wdd->ops->ping(wdd);
-   else
+   if (watchdog_hw_running(wdd) && wdd->ops->ping) {
+   err = __watchdog_ping(wdd);
+   if (err == 0)
+   set_bit(WDOG_ACTIVE, >status);
+   } else {
err = wdd->ops->start(wdd);
-   if (err == 0) {
-   set_bit(WDOG_ACTIVE, >status);
-   wd_data->last_keepalive = started_at;
-   wd_data->last_hw_keepalive = started_at;
-   watchdog_update_worker(wdd);
+   if (err == 0) {
+   set_bit(WDOG_ACTIVE, >status);
+   wd_data->last_keepalive = started_at;
+   wd_data->last_hw_keepalive = started_at;
+   watchdog_update_worker(wdd);
+   }
}
 
return err;
-- 
2.17.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH v3] arm64: dts: ti: k3-am654-main: Update otap-del-sel values

2020-06-22 Thread Tero Kristo

On 19/05/2020 11:20, Faiz Abbas wrote:

According to the latest AM65x Data Manual[1], a different output tap
delay value is optimum for a given speed mode. Update these values.

[1] http://www.ti.com/lit/gpn/am6526

Signed-off-by: Faiz Abbas 

Queued up for 5.9, thanks.

-Tero


---

v3: Updated values to the latest data manual revision

v2: Updated to the latest mainline kernel

  arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 12 +++-
  1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi 
b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
index 11887c72f23a..056130a126f9 100644
--- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
@@ -244,7 +244,17 @@
interrupts = ;
mmc-ddr-1_8v;
mmc-hs200-1_8v;
-   ti,otap-del-sel = <0x2>;
+   ti,otap-del-sel-legacy = <0x0>;
+   ti,otap-del-sel-mmc-hs = <0x0>;
+   ti,otap-del-sel-sd-hs = <0x0>;
+   ti,otap-del-sel-sdr12 = <0x0>;
+   ti,otap-del-sel-sdr25 = <0x0>;
+   ti,otap-del-sel-sdr50 = <0x8>;
+   ti,otap-del-sel-sdr104 = <0x7>;
+   ti,otap-del-sel-ddr50 = <0x5>;
+   ti,otap-del-sel-ddr52 = <0x5>;
+   ti,otap-del-sel-hs200 = <0x5>;
+   ti,otap-del-sel-hs400 = <0x0>;
ti,trm-icp = <0x8>;
dma-coherent;
};



--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH 0/2] arm64: dts: ti: k3: add platforms chipid module nodes

2020-06-22 Thread Tero Kristo

On 19/06/2020 19:26, Grygorii Strashko wrote:



On 15/06/2020 10:47, Peter Ujfalusi wrote:

Hi Grygorii,

On 13/06/2020 19.43, Grygorii Strashko wrote:

Hi Tero,

Hence k3 platforms chipid module driver was merged, there is follow 
up series

to add corresponding DT chipid nodes.

[1] https://lkml.org/lkml/2020/5/29/979

Grygorii Strashko (2):
   arm64: dts: ti: k3-am65-wakeup: add k3 platforms chipid module node
   arm64: dts: ti: k3-j721e-mcu-wakeup: add k3 platforms chipid 
module node


Can you also send a patch to enable the socinfo build?


Posted.



diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 8dd05b2a925c..1d3710e3626a 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -98,6 +98,7 @@ config ARCH_K3
  select TI_SCI_PROTOCOL
  select TI_SCI_INTR_IRQCHIP
  select TI_SCI_INTA_IRQCHIP
+    select TI_K3_SOCINFO
  help
    This enables support for Texas Instruments' K3 multicore SoC
    architecture.

With this added:
Tested-by: Peter Ujfalusi 

Tero: FYI. There is no dependecies for this series.


Queued for 5.9, thanks.

-Tero



Best regards,
grygorii


--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCHv4 1/4] dt-bindings: watchdog: Add support for TI K3 RTI watchdog

2020-06-22 Thread Tero Kristo

On 18/06/2020 19:09, Jan Kiszka wrote:

On 12.03.20 10:58, Tero Kristo wrote:

TI K3 SoCs contain an RTI (Real Time Interrupt) module which can be
used to implement a windowed watchdog functionality. Windowed watchdog
will generate an error if it is petted outside the time window, either
too early or too late.

Cc: Rob Herring 
Cc: devicet...@vger.kernel.org
Signed-off-by: Tero Kristo 
---
v4:
   * changed license to dual
   * added documentation for missing properties
   * added ref to watchdog.yaml
   * renamed main_rti0 to watchdog0 in example

  .../bindings/watchdog/ti,rti-wdt.yaml | 65 +++
  1 file changed, 65 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml

diff --git a/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml 
b/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml
new file mode 100644
index ..e83026fef2e9
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/watchdog/ti,rti-wdt.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments K3 SoC Watchdog Timer
+
+maintainers:
+  - Tero Kristo 
+
+description:
+  The TI K3 SoC watchdog timer is implemented via the RTI (Real Time
+  Interrupt) IP module. This timer adds a support for windowed watchdog
+  mode, which will signal an error if it is pinged outside the watchdog
+  time window, meaning either too early or too late. The error signal
+  generated can be routed to either interrupt a safety controller or
+  to directly reset the SoC.
+
+allOf:
+  - $ref: "watchdog.yaml#"
+
+properties:
+  compatible:
+enum:
+  - ti,j7-rti-wdt
+
+  reg:
+maxItems: 1
+
+  clocks:
+maxItems: 1
+
+  power-domains:
+maxItems: 1
+
+  assigned-clocks:
+maxItems: 1
+
+  assigned-clocks-parents:
+maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - power-domains
+
+examples:
+  - |
+/*
+ * RTI WDT in main domain on J721e SoC. Assigned clocks are used to
+ * select the source clock for the watchdog, forcing it to tick with
+ * a 32kHz clock in this case.
+ */
+#include 
+
+watchdog0: rti@220 {
+compatible = "ti,rti-wdt";


At some stage, you changed the compatible string to something
J721e-specific. This one wasn't updated.


Hmm nice catch, this should be fixed. I wonder why the DT test tools did 
not catch this when I changed the compatible...



+reg = <0x0 0x220 0x0 0x100>;
+clocks = <_clks 252 1>;
+power-domains = <_pds 252 TI_SCI_PD_EXCLUSIVE>;
+assigned-clocks = <_clks 252 1>;
+assigned-clock-parents = <_clks 252 5>;
+};



And where is the binding for the AM65x? I know that PG1 has nice
erratum, but I would expect PG2 to be fine and register-wise compatible, no?


ti,am65-rti-wdt should be added as a new compatible to this binding once 
we have a board where we can actually support this. Right now TI AM65x 
boards depend on firmware for the ESM side support; there has been some 
internal discussion about how to get this done and I believe you are 
aware of that.


-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH] crypto: sa2ul: fix odd_ptr_err.cocci warnings

2020-06-18 Thread Tero Kristo

On 18/06/2020 10:28, Herbert Xu wrote:

On Fri, Jun 12, 2020 at 11:22:02PM +0200, Julia Lawall wrote:

From: kernel test robot 

PTR_ERR should normally access the value just tested by IS_ERR

Generated by: scripts/coccinelle/tests/odd_ptr_err.cocci

Fixes: 5b8516f3bedb ("crypto: sa2ul: Add crypto driver")
CC: Keerthy 
Signed-off-by: kernel test robot 
Signed-off-by: Julia Lawall 
---

tree:   git://git.ti.com/ti-linux-kernel/ti-linux-kernel.git ti-linux-5.4.y
head:   134a1b1f8814115e2dd115b67082321bf9e63cc1
commit: 5b8516f3bedb3e1c273e7747b6e4a85c6e47907a [2369/7050] crypto: sa2ul: Add 
crypto driver
:: branch date: 3 hours ago
:: commit date: 5 months ago

  sa2ul.c |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)


This driver does not exist in the cryptodev tree.


Yeah, this is old codebase which only exist in TI internal tree at the 
moment, the driver posted upstream has seen considerable evolution (and 
is under review atm.)


-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH v2] arm64: dts: ti: k3-am654-main: Update otap-del-sel values

2020-05-15 Thread Tero Kristo

On 07/05/2020 21:15, Faiz Abbas wrote:

According to the latest AM65x Data Manual[1], a different output tap
delay value is optimum for a given speed mode. Update these values.

[1] http://www.ti.com/lit/gpn/am6526

Signed-off-by: Faiz Abbas 
---
v2: Rebased to the latest mainline kernel

  arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 12 +++-
  1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi 
b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
index 11887c72f23a..6cd9701e4ead 100644
--- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
@@ -244,7 +244,17 @@
interrupts = ;
mmc-ddr-1_8v;
mmc-hs200-1_8v;
-   ti,otap-del-sel = <0x2>;
+   ti,otap-del-sel-legacy = <0x0>;
+   ti,otap-del-sel-mmc-hs = <0x0>;
+   ti,otap-del-sel-sd-hs = <0x0>;
+   ti,otap-del-sel-sdr12 = <0x0>;
+   ti,otap-del-sel-sdr25 = <0x0>;
+   ti,otap-del-sel-sdr50 = <0x8>;
+   ti,otap-del-sel-sdr104 = <0x5>;


Isn't this wrong? Doc claims the value for sdr104 should be 0x7?

-Tero


+   ti,otap-del-sel-ddr50 = <0x5>;
+   ti,otap-del-sel-ddr52 = <0x5>;
+   ti,otap-del-sel-hs200 = <0x5>;
+   ti,otap-del-sel-hs400 = <0x0>;
ti,trm-icp = <0x8>;
dma-coherent;
};



--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH 2/6] soc: ti: omap-prm: Add basic power domain support

2020-05-14 Thread Tero Kristo

On 12/05/2020 23:38, Tony Lindgren wrote:

The PRM controller has currently only support for resets while the power
domains are still handled in the platform code.

Let's add basic power domain support to enable and disable a PRM
controlled power domain if configured in the devicetree. This can be
used for various hardware accelerators, and interconnect instances.

Further support can be added later on as needed for runtime configuration
based on domain-idle-states.

Signed-off-by: Tony Lindgren 
---
  arch/arm/mach-omap2/Kconfig |   1 +
  drivers/soc/ti/omap_prm.c   | 281 +++-
  2 files changed, 281 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -7,6 +7,7 @@ config ARCH_OMAP2
depends on ARCH_MULTI_V6
select ARCH_OMAP2PLUS
select CPU_V6
+   select PM_GENERIC_DOMAINS if PM
select SOC_HAS_OMAP2_SDRC
  
  config ARCH_OMAP3

diff --git a/drivers/soc/ti/omap_prm.c b/drivers/soc/ti/omap_prm.c
--- a/drivers/soc/ti/omap_prm.c
+++ b/drivers/soc/ti/omap_prm.c
@@ -6,18 +6,50 @@
   *Tero Kristo 
   */
  
+#include 

  #include 
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  
  #include 
  
+enum omap_prm_clocks {

+   OMAP_PRM_FCK,
+   OMAP_PRM_ICK,
+   OMAP_PRM_NR_CLOCKS,
+};
+
+enum omap_prm_domain_mode {
+   OMAP_PRMD_OFF,
+   OMAP_PRMD_RETENTION,
+   OMAP_PRMD_ON_INACTIVE,
+   OMAP_PRMD_ON_ACTIVE,
+};
+
+struct omap_prm_domain_map {
+   unsigned int usable_modes;  /* Mask of hardware supported modes */
+   unsigned long statechange:1;/* Optional low-power state change */
+   unsigned long logicretstate:1;  /* Optional logic off mode */
+};
+
+struct omap_prm_domain {
+   struct device *dev;
+   struct omap_prm *prm;
+   struct generic_pm_domain pd;
+   void __iomem *pwrstctrl;
+   void __iomem *pwrstst;


I think the pwrstst is not really used as of now, it is just part of 
couple of dev_dbg prints.



+   const struct omap_prm_domain_map *cap;
+   u32 pwrstctrl_saved;
+};
+
  struct omap_rst_map {
s8 rst;
s8 st;
@@ -27,6 +59,9 @@ struct omap_prm_data {
u32 base;
const char *name;
const char *clkdm_name;
+   u16 pwrstctrl;
+   u16 pwrstst;
+   const struct omap_prm_domain_map *dmap;
u16 rstctrl;
u16 rstst;
const struct omap_rst_map *rstmap;
@@ -36,6 +71,8 @@ struct omap_prm_data {
  struct omap_prm {
const struct omap_prm_data *data;
void __iomem *base;
+   struct clk *clocks[OMAP_PRM_NR_CLOCKS];
+   struct omap_prm_domain *prmd;
  };
  
  struct omap_reset_data {

@@ -47,6 +84,7 @@ struct omap_reset_data {
struct device *dev;
  };
  
+#define genpd_to_prm_domain(gpd) container_of(gpd, struct omap_prm_domain, pd)

  #define to_omap_reset_data(p) container_of((p), struct omap_reset_data, rcdev)
  
  #define OMAP_MAX_RESETS		8

@@ -58,6 +96,40 @@ struct omap_reset_data {
  
  #define OMAP_PRM_HAS_RESETS	(OMAP_PRM_HAS_RSTCTRL | OMAP_PRM_HAS_RSTST)
  
+#define PRM_LOGICRETSTATE	BIT(2)

+#define PRM_LOWPOWERSTATECHANGEBIT(4)
+#define PRM_POWERSTATE_MASKOMAP_PRMD_ON_ACTIVE
+
+static const struct __maybe_unused
+omap_prm_domain_map omap_prm_all = {
+   .usable_modes = BIT(OMAP_PRMD_ON_ACTIVE) | BIT(OMAP_PRMD_ON_INACTIVE) |
+   BIT(OMAP_PRMD_RETENTION) | BIT(OMAP_PRMD_OFF),
+   .statechange = 1,
+   .logicretstate = 1,
+};
+
+static const struct __maybe_unused
+omap_prm_domain_map omap_prm_noinact = {
+   .usable_modes = BIT(OMAP_PRMD_ON_ACTIVE) | BIT(OMAP_PRMD_RETENTION) |
+   BIT(OMAP_PRMD_OFF),
+   .statechange = 1,
+   .logicretstate = 1,
+};
+
+static const struct __maybe_unused
+omap_prm_domain_map omap_prm_nooff = {
+   .usable_modes = BIT(OMAP_PRMD_ON_ACTIVE) | BIT(OMAP_PRMD_ON_INACTIVE) |
+   BIT(OMAP_PRMD_RETENTION),
+   .statechange = 1,
+   .logicretstate = 1,
+};
+
+static const struct __maybe_unused
+omap_prm_domain_map omap_prm_onoff_noauto = {
+   .usable_modes = BIT(OMAP_PRMD_ON_ACTIVE) | BIT(OMAP_PRMD_OFF),
+   .statechange = 1,
+};
+
  static const struct omap_rst_map rst_map_0[] = {
{ .rst = 0, .st = 0 },
{ .rst = -1 },
@@ -151,6 +223,152 @@ static const struct of_device_id omap_prm_id_table[] = {
{ },
  };
  
+static int omap_prm_domain_power_on(struct generic_pm_domain *domain)

+{
+   struct omap_prm_domain *prmd;
+   u32 v;
+
+   prmd = genpd_to_prm_domain(domain);
+   if (!prmd->cap)
+   return 0;
+
+   dev_dbg(prmd->dev, "%s: %s: old state: pwrstctrl: %08x pwrstst: %08x\n",
+   __func__, prmd->pd.name, readl_relaxed(prmd->pwrstctrl),
+   

Re: [PATCH 1/6] dt-bindings: omap: Update PRM binding for genpd

2020-05-14 Thread Tero Kristo

On 12/05/2020 23:38, Tony Lindgren wrote:

The PRM (Power and Reset Module) has registers to enable and disable
power domains, so let's update the binding for that.

Cc: devicet...@vger.kernel.org
Cc: Rob Herring 
Signed-off-by: Tony Lindgren 
---
  Documentation/devicetree/bindings/arm/omap/prm-inst.txt | 4 
  1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/omap/prm-inst.txt 
b/Documentation/devicetree/bindings/arm/omap/prm-inst.txt
--- a/Documentation/devicetree/bindings/arm/omap/prm-inst.txt
+++ b/Documentation/devicetree/bindings/arm/omap/prm-inst.txt
@@ -18,12 +18,16 @@ Required properties:
(base address and length)
  
  Optional properties:

+- #power-domain-cells: Should be 0 if the PRM instance is a power domain.
  - #reset-cells:   Should be 1 if the PRM instance in question supports 
resets.
+- clocks: Functional and interface clocks managed by the power domain
+- clock-names: Names for the clocks using "fck" and "ick" naming


Whats the purpose of the clocks for PRM? It looks like you are using 
this with ABE domain on omap4/omap5, but why is this needed?


-Tero

  
  Example:
  
  prm_dsp2: prm@1b00 {

compatible = "ti,dra7-prm-inst", "ti,omap-prm-inst";
reg = <0x1b00 0x40>;
+   #power-domain-cells = <0>;
#reset-cells = <1>;
  };



--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


[PATCH 1/1] soc: ti: omap-prm: use atomic iopoll instead of sleeping one

2020-05-14 Thread Tero Kristo
The reset handling APIs for omap-prm can be invoked PM runtime which
runs in atomic context. For this to work properly, switch to atomic
iopoll version instead of the current which can sleep. Otherwise,
this throws a "BUG: scheduling while atomic" warning. Issue is seen
rather easily when CONFIG_PREEMPT is enabled.

Signed-off-by: Tero Kristo 
---
 drivers/soc/ti/omap_prm.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/ti/omap_prm.c b/drivers/soc/ti/omap_prm.c
index 96c6f777519c..c9b3f9ebf0bb 100644
--- a/drivers/soc/ti/omap_prm.c
+++ b/drivers/soc/ti/omap_prm.c
@@ -256,10 +256,10 @@ static int omap_reset_deassert(struct 
reset_controller_dev *rcdev,
goto exit;
 
/* wait for the status to be set */
-   ret = readl_relaxed_poll_timeout(reset->prm->base +
-reset->prm->data->rstst,
-v, v & BIT(st_bit), 1,
-OMAP_RESET_MAX_WAIT);
+   ret = readl_relaxed_poll_timeout_atomic(reset->prm->base +
+reset->prm->data->rstst,
+v, v & BIT(st_bit), 1,
+OMAP_RESET_MAX_WAIT);
if (ret)
pr_err("%s: timedout waiting for %s:%lu\n", __func__,
   reset->prm->data->name, id);
-- 
2.17.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH v3 0/2] Misc. rproc fixes around fixed memory region support

2020-05-13 Thread Tero Kristo

On 13/05/2020 02:10, Bjorn Andersson wrote:

On Fri 08 May 08:14 PDT 2020, Suman Anna wrote:


Hi Bjorn,

On 5/2/20 1:29 PM, Suman Anna wrote:

Hi Bjorn,

On 4/20/20 11:05 AM, Suman Anna wrote:

Hi Bjorn,

This is another minor revision of the fixes around fixed memory region
support [1] series. Patch 1 is revised to go back to the logic used in v1
after a long discussion on the v2 version [2]. The other suggestions can
be future improvments as they would require corresponding platform driver
changes. Please look through the discussion there and let us know your
preference. Patches are based on v5.7-rc1.

I really appreciate it if you can target the series for the current
5.7 -rc's.
The fixes would apply for all 5.1+ kernels.


Ping on these.


The patches have been reviewed and/or acked by both Mathieu and Arnaud.


Thanks for the reviews!


Can you please get these into the current -rc's?



The offending patch appeared in 5.1, so I have a hard time claiming that
this is a regression in 5.7-rc. I've added Cc: stable and picked the two
patches for 5.8.


Thanks Bjorn,

I believe 5.8 should be fine, we can backport this internally if needed.

-Tero



Thanks,
Bjorn


Thanks,
Suman



regards
Suman



Please see the v1 cover-letter [1] for the details on the issues.

regards
Suman

[1] https://patchwork.kernel.org/cover/11422723/
[2] https://patchwork.kernel.org/comment/23274389/

Suman Anna (1):
    remoteproc: Fix and restore the parenting hierarchy for vdev

Tero Kristo (1):
    remoteproc: Fall back to using parent memory pool if no dedicated
  available

   drivers/remoteproc/remoteproc_core.c   |  2 +-
   drivers/remoteproc/remoteproc_virtio.c | 12 
   2 files changed, 13 insertions(+), 1 deletion(-)







--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH v2 0/2] soc: ti: add k3 platforms chipid module driver

2020-05-07 Thread Tero Kristo

On 05/05/2020 22:34, Grygorii Strashko wrote:

Hi All,

This series introduces TI K3 Multicore SoC platforms chipid module driver
which provides identification support of the TI K3 SoCs (family, revision)
and register this information with the SoC bus. It is available under
/sys/devices/soc0/ for user space, and can be checked, where needed,
in Kernel using soc_device_match().
It is also required for introducing support for new revisions of
K3 AM65x/J721E SoCs.

Example J721E:
   # cat /sys/devices/soc0/{machine,family,revision}
   Texas Instruments K3 J721E SoC
   J721E
   SR1.0

Example AM65x:
   # cat /sys/devices/soc0/{machine,family,revision}
   Texas Instruments AM654 Base Board
   AM65X
   SR1.0

Changes in v2:
  - pr_debug() replaced with pr_info() to show SoC info on init
  - minor format change
  - split series on driver and platform changes
  - add Reviewed-by: Lokesh Vutla 

v1: https://lwn.net/Articles/818577/

Grygorii Strashko (2):
   dt-bindings: soc: ti: add binding for k3 platforms chipid module
   soc: ti: add k3 platforms chipid module driver

  .../bindings/soc/ti/k3-socinfo.yaml   |  40 ++
  drivers/soc/ti/Kconfig|  10 ++
  drivers/soc/ti/Makefile   |   1 +
  drivers/soc/ti/k3-socinfo.c   | 135 ++
  4 files changed, 186 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/soc/ti/k3-socinfo.yaml
  create mode 100644 drivers/soc/ti/k3-socinfo.c



Got a minor comments on patch #2, other than that looks fine to me. Once 
that is fixed, for whole series:


Reviewed-by: Tero Kristo 
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH v2 2/2] soc: ti: add k3 platforms chipid module driver

2020-05-07 Thread Tero Kristo

On 05/05/2020 22:34, Grygorii Strashko wrote:

The Texas Instruments K3 Multicore SoC platforms have chipid module which
is represented by CTRLMMR_xxx_JTAGID register and contains information
about SoC id and revision.
  Bits:
   31-28 VARIANT Device variant
   27-12 PARTNO  Part number
   11-1  MFG Indicates TI as manufacturer (0x17)
   1 Always 1

This patch adds corresponding driver to identify the TI K3 SoC family and
revision, and registers this information with the SoC bus. It is available
under /sys/devices/soc0/ for user space, and can be checked, where needed,
in Kernel using soc_device_match().

Identification is done by:
- checking MFG to be TI ID
  - retrieving Device variant (revision)
  - retrieving Part number and convert it to the family
  - retrieving machine from DT "/model"

Example J721E:
   # cat /sys/devices/soc0/{machine,family,revision}
   Texas Instruments K3 J721E SoC
   J721E
   SR1.0

Example AM65x:
   # cat /sys/devices/soc0/{machine,family,revision}
   Texas Instruments AM654 Base Board
   AM65X
   SR1.0

Signed-off-by: Grygorii Strashko 
Reviewed-by: Lokesh Vutla 
---
  drivers/soc/ti/Kconfig  |  10 +++
  drivers/soc/ti/Makefile |   1 +
  drivers/soc/ti/k3-socinfo.c | 135 
  3 files changed, 146 insertions(+)
  create mode 100644 drivers/soc/ti/k3-socinfo.c

diff --git a/drivers/soc/ti/Kconfig b/drivers/soc/ti/Kconfig
index 4486e055794c..e192fb788836 100644
--- a/drivers/soc/ti/Kconfig
+++ b/drivers/soc/ti/Kconfig
@@ -91,6 +91,16 @@ config TI_K3_RINGACC
  and a consumer. There is one RINGACC module per NAVSS on TI AM65x SoCs
  If unsure, say N.
  
+config TI_K3_SOCINFO

+   bool
+   depends on ARCH_K3 || COMPILE_TEST
+   select SOC_BUS
+   select MFD_SYSCON
+   help
+ Include support for the SoC bus socinfo for the TI K3 Multicore SoC
+ platforms to provide information about the SoC family and
+ variant to user space.
+
  endif # SOC_TI
  
  config TI_SCI_INTA_MSI_DOMAIN

diff --git a/drivers/soc/ti/Makefile b/drivers/soc/ti/Makefile
index bec827937a5f..1110e5c98685 100644
--- a/drivers/soc/ti/Makefile
+++ b/drivers/soc/ti/Makefile
@@ -11,3 +11,4 @@ obj-$(CONFIG_WKUP_M3_IPC) += wkup_m3_ipc.o
  obj-$(CONFIG_TI_SCI_PM_DOMAINS)   += ti_sci_pm_domains.o
  obj-$(CONFIG_TI_SCI_INTA_MSI_DOMAIN)  += ti_sci_inta_msi.o
  obj-$(CONFIG_TI_K3_RINGACC)   += k3-ringacc.o
+obj-$(CONFIG_TI_K3_SOCINFO)+= k3-socinfo.o
diff --git a/drivers/soc/ti/k3-socinfo.c b/drivers/soc/ti/k3-socinfo.c
new file mode 100644
index ..4c21e099d3c7
--- /dev/null
+++ b/drivers/soc/ti/k3-socinfo.c
@@ -0,0 +1,135 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * TI K3 SoC info driver
+ *
+ * Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define CTRLMMR_WKUP_JTAGID_REG0
+/*
+ * Bits:
+ *  31-28 VARIANT  Device variant
+ *  27-12 PARTNO   Part number
+ *  11-1  MFG  Indicates TI as manufacturer (0x17)
+ *  1  Always 1
+ */
+#define CTRLMMR_WKUP_JTAGID_VARIANT_SHIFT  (28)
+#define CTRLMMR_WKUP_JTAGID_VARIANT_MASK   GENMASK(31, 28)
+
+#define CTRLMMR_WKUP_JTAGID_PARTNO_SHIFT   (12)
+#define CTRLMMR_WKUP_JTAGID_PARTNO_MASKGENMASK(27, 12)
+
+#define CTRLMMR_WKUP_JTAGID_MFG_SHIFT  (1)
+#define CTRLMMR_WKUP_JTAGID_MFG_MASK   GENMASK(11, 1)
+
+#define CTRLMMR_WKUP_JTAGID_MFG_TI 0x17
+
+static const struct k3_soc_id {
+   unsigned int id;
+   const char *family_name;
+} k3_soc_ids[] = {
+   { 0xBB5A, "AM65X" },
+   { 0xBB64, "J721E" },
+};
+
+static int __init partno_to_names(unsigned int partno,
+ struct soc_device_attribute *soc_dev_attr)
+{
+   int i;
+
+   for (i = 0; i < ARRAY_SIZE(k3_soc_ids); i++)
+   if (partno == k3_soc_ids[i].id) {
+   soc_dev_attr->family = k3_soc_ids[i].family_name;
+   return 0;
+   }
+
+   return -EINVAL;
+}
+
+static int __init k3_chipinfo_init(void)
+{
+   struct soc_device_attribute *soc_dev_attr;
+   struct soc_device *soc_dev;
+   struct device_node *node;
+   struct regmap *regmap;
+   u32 partno_id;
+   u32 variant;
+   u32 jtag_id;
+   u32 mfg;
+   int ret;
+
+   node = of_find_compatible_node(NULL, NULL, "ti,am654-chipid");
+   if (!node)
+   return -ENODEV;
+
+   regmap = device_node_to_regmap(node);
+   of_node_put(node);
+
+   if (IS_ERR(regmap))
+   return PTR_ERR(regmap);
+
+   ret = regmap_read(regmap, CTRLMMR_WKUP_JTAGID_REG, _id);
+   if (ret < 0)
+   return ret;
+
+   mfg = (jtag_id & CTRLMMR_WKUP_JTAGID_MFG_MASK) >>
+

Re: [Patch 1/3] ARM: dts: am43xx: add support for clkout1 clock

2019-10-22 Thread Tero Kristo

On 22/10/2019 19:21, Benoit Parrot wrote:

Tony Lindgren  wrote on Tue [2019-Oct-22 08:48:16 -0700]:

* Benoit Parrot  [191016 18:47]:

--- a/arch/arm/boot/dts/am43xx-clocks.dtsi
+++ b/arch/arm/boot/dts/am43xx-clocks.dtsi
@@ -704,6 +704,60 @@
ti,bit-shift = <8>;
reg = <0x2a48>;
};
+
+   clkout1_osc_div_ck: clkout1_osc_div_ck {
+   #clock-cells = <0>;
+   compatible = "ti,divider-clock";
+   clocks = <_clkin_ck>;
+   ti,bit-shift = <20>;
+   ti,max-div = <4>;
+   reg = <0x4100>;
+   };


Here too please describe why the clock names are not generic.


Tero originally had this patch in the kernel so this is somewhat of a
revert. Since these "clock" were removed. If the name syntax is no longer
valid for some reason, then I will need a little more informations to
proceed.

Tero, can you assist here?


This one is just following the naming convention of the rest of the 
clocks atm.


If we need to fix all the underscore name clocks, that requires pretty 
much complete revamp of both the dts data + clock data under the clock 
driver, and it is not backwards compatible either. How should we tackle 
that one?


We could maybe add support code in kernel to do s/-/_/g for the "new" 
clocks so that their parent-child relationships would be retained, and 
then convert the clocks in phases.


-Tero



Benoit



Regards,

Tony


--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH 0/2] Add Support for MMC/SD for J721e-base-board

2019-10-18 Thread Tero Kristo

On 09/10/2019 12:57, Faiz Abbas wrote:

Hi,

On 19/09/19 9:02 PM, Faiz Abbas wrote:

The following are dts patches to add MMC/SD Support on TI's J721e base
board.

Patches depend on Lokesh's gpio patches[1] and device exclusivity patches[2].

[1] https://patchwork.kernel.org/cover/11085643/
[2] https://patchwork.kernel.org/cover/11051559/

Faiz Abbas (2):
   arm64: dts: ti: j721e-main: Add SDHCI nodes
   arm64: dts: ti: j721e-common-proc-board: Add Support for eMMC and SD
 card

  .../dts/ti/k3-j721e-common-proc-board.dts | 34 +
  arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 50 +++
  2 files changed, 84 insertions(+)



Gentle ping.


Queuing up towards 5.5, thanks.

-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH] arm64: dts: ti: k3-am654-base-board: Add disable-wp for mmc0

2019-10-18 Thread Tero Kristo

On 03/10/2019 14:42, Faiz Abbas wrote:

MMC0_SDWP is not connected to the card. Indicate this by adding a
disable-wp flag.

Signed-off-by: Faiz Abbas 
---
  arch/arm64/boot/dts/ti/k3-am654-base-board.dts | 1 +
  1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts 
b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
index 1102b84f853d..143474119328 100644
--- a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
+++ b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
@@ -221,6 +221,7 @@
bus-width = <8>;
non-removable;
ti,driver-strength-ohm = <50>;
+   disable-wp;
  };
  
  _1 {




Queuing up towards 5.5, thanks.

-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: Lay common foundation to make PVR/SGX work without hacks on OMAP34xx, OMAP36xx, AM335x and potentially OMAP4, OMAP5

2019-10-09 Thread Tero Kristo

On 09/10/2019 17:23, H. Nikolaus Schaller wrote:



Am 09.10.2019 um 15:55 schrieb Tero Kristo :

On 09/10/2019 15:53, H. Nikolaus Schaller wrote:

Am 08.10.2019 um 22:15 schrieb H. Nikolaus Schaller :


But I can't access the sgx registers and get memory faults. Maybe
my script has a bug and is trying the wrong address. Have to check
with some distance...

Now I have done more tests on am335x. It is not my script but something else.
Trying to read 0x5600fe00 after doing
echo on > /sys/bus/platform/devices/5600fe00.target-module/power/control
gives page faults.
When trying to load the kernel driver, the omap_reset_deassert message has
gone but the driver does no initialize:
root@letux:~# modprobe pvrsrvkm_omap_am335x_sgx530_125
[   45.774712] pvrsrvkm_omap_am335x_sgx530_125: module is from the staging 
directory, the quality is unknown, you have been warned.
root@letux:~#
Here is the CM/PM register dump after enabling power/control
*** SGX Register Dump ***
0x44E00900 0301 CM_GFX_L3_CLKSTCTRL
0x44E00904 0005 CM_GFX_GFX_CLKCTRL
0x44E0090c 0002 CM_GFX_L4LS_GFX_CLKSTCTR
0x44E00910 0003 CM_GFX_MMUCFG_CLKCTRL
0x44E00914 0003 CM_GFX_MMUDATA_CLKCTRL
0x44E0052c  CM_DPLL.CLKSEL_GFX_FCLK
0x44E01100 00060047 PM_GFX_PWRSTCTRL
0x44E01104 0001 RM_GFX_RSTCTRL
0x44E01110 0037 PM_GFX_PWRSTST


Are you sure you have the graphics node properly applied in your kernel?


Not really... There are several patch sets which seem to be necessary
(to support all omap variants) and I am not sure if I have them all and 
correctly.

I have collected these patches on top of v5.4-rc2:

272d7200c77a ARM: dts: omap5: fix gpu_cm clock provider name
96fa23010f2a dt-bindings: omap: add new binding for PRM instances
a164172c1f40 soc: ti: add initial PRM driver with reset control support
42a5e4261993 soc: ti: omap-prm: poll for reset complete during de-assert
9237f39716be soc: ti: omap-prm: add support for denying idle for reset 
clockdomain
bf2ae22e5bcf soc: ti: omap-prm: add omap4 PRM data
be5cb64f10e0 soc: ti: omap-prm: add data for am33xx
86646d7d79be soc: ti: omap-prm: add dra7 PRM data
c3b5455dfd65 soc: ti: omap-prm: add am4 PRM data
e26d4ff7ad15 soc: ti: omap-prm: add omap5 PRM data
66369100d1fc clk: ti: am43xx: drop idlest polling from gfx clock


You should have similar patch as above for am33xx. Otherwise it will 
probably fail probing the ti-sysc, resulting in the failure you see.


-Tero


d96899e143de bus: ti-sysc: re-order reset and main clock controls
45071446bd05 bus: ti-sysc: drop the extra hardreset during init
0da134c3aa9d bus: ti-sysc: avoid toggling power state of module during probe
17b70c96b539 ARM: OMAP2+: pdata-quirks: add PRM data for reset support
af81a68c65d7 clk: ti: clkctrl: fix setting up clkctrl clocks
d7dd7f44bce4 clk: ti: clkctrl: convert to use bit helper macros instead of 
bitops
42ee8270adfd clk: ti: clkctrl: add new exported API for checking standby info
218b39a8c851 dt-bindings: clk: add omap5 iva clkctrl definitions
41b6c466efde clk: ti: omap5: add IVA subsystem clkctrl data
38cfdebcc2f8 clk: ti: dra7xx: Drop idlest polling from IPU & DSP clkctrl clocks
39e827b0dfe5 clk: ti: omap4: Drop idlest polling from IPU & DSP clkctrl clocks
f4584f1e5bff clk: ti: omap5: Drop idlest polling from IPU & DSP clkctrl clocks
1c7f5871e5a0 clk: ti: am43xx: drop idlest polling from pruss clkctrl clock
53363c4cfb3d clk: ti: am33xx: drop idlest polling from pruss clkctrl clock
1907994ee9ce ARM: dts: omap5: add IVA clkctrl nodes
8182f3f282de ARM: dts: dra7: add PRM nodes
ff2880fb99c7 ARM: dts: omap4: add PRM nodes
4d49da48c458 ARM: dts: am33xx: Add PRM data
325cffda2b2d ARM: dts: am43xx: Add PRM data
b6ceeb4c5b74 ARM: dts: omap5: Add PRM data
303b7b4bcc60 ARM: dts: dra7: convert IOMMUs to use ti-sysc
d875126d92f7 ARM: dts: dra74x: convert IOMMUs to use ti-sysc
8f699534deb8 ARM: dts: omap4: convert IOMMUs to use ti-sysc
b1ec9e25a686 ARM: dts: omap5: convert IOMMUs to use ti-sysc
e90c0cc1e4a5 ARM: dts: Configure rstctrl reset for SGX



As you can see the RM_GFX_RSTCTRL is still asserted (it should be zero so that 
you can access the module) and the CM_GFX_GFX_CLKCTRL is also disabled (bits 
0-1 are 0, should be 2 in the working case.)


Ok.



It works for me with my branch + the single am33xx patch from Tony.


Is there a link so that I can compare with the right version?

BR and thanks,
Nikolaus



--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: Lay common foundation to make PVR/SGX work without hacks on OMAP34xx, OMAP36xx, AM335x and potentially OMAP4, OMAP5

2019-10-09 Thread Tero Kristo

On 09/10/2019 15:53, H. Nikolaus Schaller wrote:



Am 08.10.2019 um 22:15 schrieb H. Nikolaus Schaller :



Am 08.10.2019 um 10:00 schrieb Tero Kristo :

On 07/10/2019 22:24, H. Nikolaus Schaller wrote:

Hi Tero,

Am 07.10.2019 um 21:18 schrieb Tero Kristo :

On 07/10/2019 18:52, Tony Lindgren wrote:

Hi,
* H. Nikolaus Schaller  [191005 16:59]:
Please try with Tero's current github branch at github.com/t-kristo/linux-pm.git
5.4-rc1-ipc from few days ago, the earlier versions had still issues.


Yeah, this one should be fixed now.

Ok! Will try asap.



* OMAP5 (Pyra): fails to enable the clocks (did work with the previous version)
[  304.140363] clock-controller:clk::0: failed to enable
[  304.147388] PVR_K:(Error): EnableSGXClocks: pm_runtime_get_sync failed (16)

Hmm no idea what might be up with this one. Did some clkctrl clock
fixes maybe cause a regression here? Tero do you have any ideas?


So, this one I am not too sure, I haven't looked at omap5 graphics clocking. I 
don't think it has anything to do with reset handling though.

Is there some simple way to try this out on board; without PVR module that is?

Yes, I have also seen it when just running the commands in the original commit 
message [1]:
# echo on > $(find /sys -name control | grep \/5600)
# rwmem 0x5600fe00  # OCP Revision
0x5600fe00 = 0x4000
# echo auto > $(find /sys -name control | grep \/5600)
# rwmem 0x5600fe10
# rwmem 0x5624
But I have not yet tested with 5.4-rc2, just 5.4-rc1.


Ok, there is a one liner DTS data fix for this issue, attached.


Yes, have tested and it fixes omap5. I have the 3D demo running again on the 
Pyra. Yay!

Together with the latest rstcrtl patches, am335x is now better.
No omap_reset_deassert: timedout waiting for gfx:0 any more.

But I can't access the sgx registers and get memory faults. Maybe
my script has a bug and is trying the wrong address. Have to check
with some distance...


Now I have done more tests on am335x. It is not my script but something else.

Trying to read 0x5600fe00 after doing

echo on > /sys/bus/platform/devices/5600fe00.target-module/power/control

gives page faults.

When trying to load the kernel driver, the omap_reset_deassert message has
gone but the driver does no initialize:

root@letux:~# modprobe pvrsrvkm_omap_am335x_sgx530_125
[   45.774712] pvrsrvkm_omap_am335x_sgx530_125: module is from the staging 
directory, the quality is unknown, you have been warned.
root@letux:~#

Here is the CM/PM register dump after enabling power/control

*** SGX Register Dump ***
0x44E00900 0301 CM_GFX_L3_CLKSTCTRL
0x44E00904 0005 CM_GFX_GFX_CLKCTRL
0x44E0090c 0002 CM_GFX_L4LS_GFX_CLKSTCTR
0x44E00910 0003 CM_GFX_MMUCFG_CLKCTRL
0x44E00914 0003 CM_GFX_MMUDATA_CLKCTRL
0x44E0052c  CM_DPLL.CLKSEL_GFX_FCLK
0x44E01100 00060047 PM_GFX_PWRSTCTRL
0x44E01104 0001 RM_GFX_RSTCTRL
0x44E01110 0037 PM_GFX_PWRSTST


Are you sure you have the graphics node properly applied in your kernel?

As you can see the RM_GFX_RSTCTRL is still asserted (it should be zero 
so that you can access the module) and the CM_GFX_GFX_CLKCTRL is also 
disabled (bits 0-1 are 0, should be 2 in the working case.)


It works for me with my branch + the single am33xx patch from Tony.

-Tero



BR,
Nikolaus




--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH v3 08/14] dmaengine: ti: New driver for K3 UDMA - split#1: defines, structs, io func

2019-10-09 Thread Tero Kristo

On 01/10/2019 09:16, Peter Ujfalusi wrote:

Split patch for review containing: defines, structs, io and low level
functions and interrupt callbacks.

DMA driver for
Texas Instruments K3 NAVSS Unified DMA – Peripheral Root Complex (UDMA-P)

The UDMA-P is intended to perform similar (but significantly upgraded) functions
as the packet-oriented DMA used on previous SoC devices. The UDMA-P module
supports the transmission and reception of various packet types. The UDMA-P is
architected to facilitate the segmentation and reassembly of SoC DMA data
structure compliant packets to/from smaller data blocks that are natively
compatible with the specific requirements of each connected peripheral. Multiple
Tx and Rx channels are provided within the DMA which allow multiple segmentation
or reassembly operations to be ongoing. The DMA controller maintains state
information for each of the channels which allows packet segmentation and
reassembly operations to be time division multiplexed between channels in order
to share the underlying DMA hardware. An external DMA scheduler is used to
control the ordering and rate at which this multiplexing occurs for Transmit
operations. The ordering and rate of Receive operations is indirectly controlled
by the order in which blocks are pushed into the DMA on the Rx PSI-L interface.

The UDMA-P also supports acting as both a UTC and UDMA-C for its internal
channels. Channels in the UDMA-P can be configured to be either Packet-Based or
Third-Party channels on a channel by channel basis.

The initial driver supports:
- MEM_TO_MEM (TR mode)
- DEV_TO_MEM (Packet / TR mode)
- MEM_TO_DEV (Packet / TR mode)
- Cyclic (Packet / TR mode)
- Metadata for descriptors

Signed-off-by: Peter Ujfalusi 


Did review this to best of my ability but could not find anything 
obviously broken, thus:


Reviewed-by: Tero Kristo 

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH v3 05/14] dmaengine: Add support for reporting DMA cached data amount

2019-10-09 Thread Tero Kristo

On 01/10/2019 09:16, Peter Ujfalusi wrote:

A DMA hardware can have big cache or FIFO and the amount of data sitting in
the DMA fabric can be an interest for the clients.

For example in audio we want to know the delay in the data flow and in case
the DMA have significantly large FIFO/cache, it can affect the latenc/delay

Signed-off-by: Peter Ujfalusi 


Reviewed-by: Tero Kristo 


---
  drivers/dma/dmaengine.h   | 8 
  include/linux/dmaengine.h | 2 ++
  2 files changed, 10 insertions(+)

diff --git a/drivers/dma/dmaengine.h b/drivers/dma/dmaengine.h
index 501c0b063f85..b0b97475707a 100644
--- a/drivers/dma/dmaengine.h
+++ b/drivers/dma/dmaengine.h
@@ -77,6 +77,7 @@ static inline enum dma_status dma_cookie_status(struct 
dma_chan *chan,
state->last = complete;
state->used = used;
state->residue = 0;
+   state->in_flight_bytes = 0;
}
return dma_async_is_complete(cookie, complete, used);
  }
@@ -87,6 +88,13 @@ static inline void dma_set_residue(struct dma_tx_state 
*state, u32 residue)
state->residue = residue;
  }
  
+static inline void dma_set_in_flight_bytes(struct dma_tx_state *state,

+  u32 in_flight_bytes)
+{
+   if (state)
+   state->in_flight_bytes = in_flight_bytes;
+}
+
  struct dmaengine_desc_callback {
dma_async_tx_callback callback;
dma_async_tx_callback_result callback_result;
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 40d062c3b359..02ceef95340a 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -682,11 +682,13 @@ static inline struct dma_async_tx_descriptor 
*txd_next(struct dma_async_tx_descr
   * @residue: the remaining number of bytes left to transmit
   *on the selected transfer for states DMA_IN_PROGRESS and
   *DMA_PAUSED if this is implemented in the driver, else 0
+ * @in_flight_bytes: amount of data in bytes cached by the DMA.
   */
  struct dma_tx_state {
dma_cookie_t last;
dma_cookie_t used;
u32 residue;
+   u32 in_flight_bytes;
  };
  
  /**




--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH v3 06/14] dmaengine: ti: Add cppi5 header for UDMA

2019-10-09 Thread Tero Kristo

On 01/10/2019 09:16, Peter Ujfalusi wrote:

It might be good to add some sort of description for the patch. At least 
telling what cppi5 actually is and why it is needed for UDMA.


Other than that, looks fine to me.

-Tero


Signed-off-by: Peter Ujfalusi 
---
  include/linux/dma/ti-cppi5.h | 1049 ++
  1 file changed, 1049 insertions(+)
  create mode 100644 include/linux/dma/ti-cppi5.h

diff --git a/include/linux/dma/ti-cppi5.h b/include/linux/dma/ti-cppi5.h
new file mode 100644
index ..f795f8cb7cc5
--- /dev/null
+++ b/include/linux/dma/ti-cppi5.h
@@ -0,0 +1,1049 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * CPPI5 descriptors interface
+ *
+ * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com
+ */
+
+#ifndef __TI_CPPI5_H__
+#define __TI_CPPI5_H__
+
+#include 
+#include 
+#include 
+
+/**
+ * struct cppi5_desc_hdr_t - Descriptor header, present in all types of
+ *  descriptors
+ * @pkt_info0: Packet info word 0 (n/a in Buffer desc)
+ * @pkt_info0: Packet info word 1 (n/a in Buffer desc)
+ * @pkt_info0: Packet info word 2 (n/a in Buffer desc)


You use @pkt_info0 for all, should be @pkt_info0,1,2.


+ * @src_dst_tag:   Packet info word 3 (n/a in Buffer desc)
+ */
+struct cppi5_desc_hdr_t {
+   u32 pkt_info0;
+   u32 pkt_info1;
+   u32 pkt_info2;
+   u32 src_dst_tag;
+} __packed;
+
+/**
+ * struct cppi5_host_desc_t - Host-mode packet and buffer descriptor definition
+ * @hdr:   Descriptor header
+ * @next_desc: word 4/5: Linking word
+ * @buf_ptr:   word 6/7: Buffer pointer
+ * @buf_info1: word 8: Buffer valid data length
+ * @org_buf_len:   word 9: Original buffer length
+ * @org_buf_ptr:   word 10/11: Original buffer pointer
+ * @epib[0]:   Extended Packet Info Data (optional, 4 words), and/or
+ * Protocol Specific Data (optional, 0-128 bytes in
+ * multiples of 4), and/or
+ * Other Software Data (0-N bytes, optional)
+ */
+struct cppi5_host_desc_t {
+   struct cppi5_desc_hdr_t hdr;
+   u64 next_desc;
+   u64 buf_ptr;
+   u32 buf_info1;
+   u32 org_buf_len;
+   u64 org_buf_ptr;
+   u32 epib[0];
+} __packed;
+
+#define CPPI5_DESC_MIN_ALIGN   (16U)
+
+#define CPPI5_INFO0_HDESC_EPIB_SIZE(16U)
+#define CPPI5_INFO0_HDESC_PSDATA_MAX_SIZE  (128U)
+
+#define CPPI5_INFO0_HDESC_TYPE_SHIFT   (30U)
+#define CPPI5_INFO0_HDESC_TYPE_MASKGENMASK(31, 30)
+#define   CPPI5_INFO0_DESC_TYPE_VAL_HOST   (1U)
+#define   CPPI5_INFO0_DESC_TYPE_VAL_MONO   (2U)
+#define   CPPI5_INFO0_DESC_TYPE_VAL_TR (3U)
+#define CPPI5_INFO0_HDESC_EPIB_PRESENT BIT(29)
+/*
+ * Protocol Specific Words location:
+ * 0 - located in the descriptor,
+ * 1 = located in the SOP Buffer immediately prior to the data.
+ */
+#define CPPI5_INFO0_HDESC_PSINFO_LOCATION  BIT(28)
+#define CPPI5_INFO0_HDESC_PSINFO_SIZE_SHIFT(22U)
+#define CPPI5_INFO0_HDESC_PSINFO_SIZE_MASK GENMASK(27, 22)
+#define CPPI5_INFO0_HDESC_PKTLEN_SHIFT (0)
+#define CPPI5_INFO0_HDESC_PKTLEN_MASK  GENMASK(21, 0)
+
+#define CPPI5_INFO1_DESC_PKTERROR_SHIFT(28U)
+#define CPPI5_INFO1_DESC_PKTERROR_MASK GENMASK(31, 28)
+#define CPPI5_INFO1_HDESC_PSFLGS_SHIFT (24U)
+#define CPPI5_INFO1_HDESC_PSFLGS_MASK  GENMASK(27, 24)
+#define CPPI5_INFO1_DESC_PKTID_SHIFT   (14U)
+#define CPPI5_INFO1_DESC_PKTID_MASKGENMASK(23, 14)
+#define CPPI5_INFO1_DESC_FLOWID_SHIFT  (0)
+#define CPPI5_INFO1_DESC_FLOWID_MASK   GENMASK(13, 0)
+
+#define CPPI5_INFO2_HDESC_PKTTYPE_SHIFT(27U)
+#define CPPI5_INFO2_HDESC_PKTTYPE_MASK GENMASK(31, 27)
+/* Return Policy: 0 - Entire packet 1 - Each buffer */
+#define CPPI5_INFO2_HDESC_RETPOLICYBIT(18)
+/*
+ * Early Return:
+ * 0 = desc pointers should be returned after all reads have been completed
+ * 1 = desc pointers should be returned immediately upon fetching
+ * the descriptor and beginning to transfer data.
+ */
+#define CPPI5_INFO2_HDESC_EARLYRET BIT(17)
+/*
+ * Return Push Policy:
+ * 0 = Descriptor must be returned to tail of queue
+ * 1 = Descriptor must be returned to head of queue
+ */
+#define CPPI5_INFO2_DESC_RETPUSHPOLICY BIT(16)
+#define CPPI5_INFO2_DESC_RETQ_SHIFT(0)
+#define CPPI5_INFO2_DESC_RETQ_MASK GENMASK(15, 0)
+
+#define CPPI5_INFO3_DESC_SRCTAG_SHIFT  (16U)
+#define CPPI5_INFO3_DESC_SRCTAG_MASK   GENMASK(31, 16)
+#define CPPI5_INFO3_DESC_DSTTAG_SHIFT  (0)
+#define CPPI5_INFO3_DESC_DSTTAG_MASK   GENMASK(15, 0)
+
+#define CPPI5_BUFINFO1_HDESC_DATA_LEN_SHIFT(0)
+#define CPPI5_BUFINFO1_HDESC_DATA_LEN_MASK GENMASK(27, 0)
+
+#define CPPI5_OBUFINFO0_HDESC_BUF_LEN_SHIFT(0)
+#define 

Re: [PATCH v3 04/14] dmaengine: Add metadata_ops for dma_async_tx_descriptor

2019-10-09 Thread Tero Kristo

On 01/10/2019 09:16, Peter Ujfalusi wrote:

The metadata is best described as side band data or parameters traveling
alongside the data DMAd by the DMA engine. It is data
which is understood by the peripheral and the peripheral driver only, the
DMA engine see it only as data block and it is not interpreting it in any
way.

The metadata can be different per descriptor as it is a parameter for the
data being transferred.

If the DMA supports per descriptor metadata it can implement the attach,
get_ptr/set_len callbacks.

Client drivers must only use either attach or get_ptr/set_len to avoid
misconfiguration.

Client driver can check if a given metadata mode is supported by the
channel during probe time with
dmaengine_is_metadata_mode_supported(chan, DESC_METADATA_CLIENT);
dmaengine_is_metadata_mode_supported(chan, DESC_METADATA_ENGINE);

and based on this information can use either mode.

Wrappers are also added for the metadata_ops.

To be used in DESC_METADATA_CLIENT mode:
dmaengine_desc_attach_metadata()

To be used in DESC_METADATA_ENGINE mode:
dmaengine_desc_get_metadata_ptr()
dmaengine_desc_set_metadata_len()

Signed-off-by: Peter Ujfalusi 


Again couple of typos below, but other than that:

Reviewed-by: Tero Kristo 


---
  drivers/dma/dmaengine.c   |  73 ++
  include/linux/dmaengine.h | 108 ++





+ * @DESC_METADATA_ENGINE - the metadata buffer is allocated/managed by the DMA
+ *  driver. The client driver can ask for the pointer, maximum size and the
+ *  currently used size of the metadata and can directly update or read it.
+ *  dmaengine_desc_get_metadata_ptr() and dmaengine_desc_set_metadata_len() is
+ *  provided as helper functions.
+ *
+ * Client drivers interested to use this mode can follow:
+ * - DMA_MEM_TO_DEV / DEV_MEM_TO_MEM:
+ *   1. prepare the descriptor (dmaengine_prep_*)
+ *   2. use dmaengine_desc_get_metadata_ptr() to get the pointer to the 
engine's
+ * metadata area
+ *   3. update the metadata at the pointer
+ *   4. use dmaengine_desc_set_metadata_len()  to tell the DMA engine the 
amount
+ * of data the client has placed into the metadata buffer
+ *   5. submit the transfer
+ * - DMA_DEV_TO_MEM:
+ *   1. prepare the descriptor (dmaengine_prep_*)
+ *   2. submit the transfer
+ *   3. on transfer completion, use dmaengine_desc_get_metadata_ptr() to get 
the
+ * pointer to the engine's metadata are


are = area?


+ *   4. Read out the metadate from the pointer


metadate = metadata?

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: [PATCH v3 03/14] dmaengine: doc: Add sections for per descriptor metadata support

2019-10-09 Thread Tero Kristo

On 01/10/2019 09:16, Peter Ujfalusi wrote:

Update the provider and client documentation with details about the
metadata support.

Signed-off-by: Peter Ujfalusi 


Couple of typos below, but they don't really change the readability of 
the document so:


Reviewed-by: Tero Kristo 


---
  Documentation/driver-api/dmaengine/client.rst | 75 +++
  .../driver-api/dmaengine/provider.rst | 46 
  2 files changed, 121 insertions(+)

diff --git a/Documentation/driver-api/dmaengine/client.rst 
b/Documentation/driver-api/dmaengine/client.rst
index 45953f171500..d708e46b88a2 100644
--- a/Documentation/driver-api/dmaengine/client.rst
+++ b/Documentation/driver-api/dmaengine/client.rst
@@ -151,6 +151,81 @@ The details of these operations are:
   Note that callbacks will always be invoked from the DMA
   engines tasklet, never from interrupt context.
  
+  Optional: per descriptor metadata

+  -
+  DMAengine provides two ways for metadata support.
+
+  DESC_METADATA_CLIENT
+
+The metadata buffer is allocated/provided by the client driver and it is
+attached to the descriptor.
+
+  .. code-block:: c
+
+ int dmaengine_desc_attach_metadata(struct dma_async_tx_descriptor *desc,
+  void *data, size_t len);
+
+  DESC_METADATA_ENGINE
+
+The metadata buffer is allocated/managed by the DMA driver. The client
+driver can ask for the pointer, maximum size and the currently used size of
+the metadata and can directly update or read it.
+
+  .. code-block:: c
+
+ void *dmaengine_desc_get_metadata_ptr(struct dma_async_tx_descriptor 
*desc,
+   size_t *payload_len, size_t *max_len);
+
+ int dmaengine_desc_set_metadata_len(struct dma_async_tx_descriptor *desc,
+   size_t payload_len);
+
+  Client drivers can query if a given mode is supported with:
+
+  .. code-block:: c
+
+ bool dmaengine_is_metadata_mode_supported(struct dma_chan *chan,
+   enum dma_desc_metadata_mode mode);
+
+  Depending on the used mode client drivers must follow different flow.
+
+  DESC_METADATA_CLIENT
+
+- DMA_MEM_TO_DEV / DEV_MEM_TO_MEM:
+  1. prepare the descriptor (dmaengine_prep_*)
+ construct the metadata in the client's buffer
+  2. use dmaengine_desc_attach_metadata() to attach the buffer to the
+ descriptor
+  3. submit the transfer
+- DMA_DEV_TO_MEM:
+  1. prepare the descriptor (dmaengine_prep_*)
+  2. use dmaengine_desc_attach_metadata() to attach the buffer to the
+ descriptor
+  3. submit the transfer
+  4. when the transfer is completed, the metadata should be available in 
the
+ attached buffer
+
+  DESC_METADATA_ENGINE
+
+- DMA_MEM_TO_DEV / DEV_MEM_TO_MEM:
+  1. prepare the descriptor (dmaengine_prep_*)
+  2. use dmaengine_desc_get_metadata_ptr() to get the pointer to the
+ engine's metadata area
+  3. update the metadata at the pointer
+  4. use dmaengine_desc_set_metadata_len()  to tell the DMA engine the
+ amount of data the client has placed into the metadata buffer
+  5. submit the transfer
+- DMA_DEV_TO_MEM:
+  1. prepare the descriptor (dmaengine_prep_*)
+  2. submit the transfer
+  3. on transfer completion, use dmaengine_desc_get_metadata_ptr() to get 
the
+ pointer to the engine's metadata are


are = area?


+  4. Read out the metadate from the pointer


metadate = metadata?


+
+  .. note::
+
+ Mixed use of DESC_METADATA_CLIENT / DESC_METADATA_ENGINE is not allowed,
+ client drivers must use either of the modes per descriptor.
+
  4. Submit the transaction
  
 Once the descriptor has been prepared and the callback information

diff --git a/Documentation/driver-api/dmaengine/provider.rst 
b/Documentation/driver-api/dmaengine/provider.rst
index dfc4486b5743..9e6d87b3c477 100644
--- a/Documentation/driver-api/dmaengine/provider.rst
+++ b/Documentation/driver-api/dmaengine/provider.rst
@@ -247,6 +247,52 @@ after each transfer. In case of a ring buffer, they may 
loop
  (DMA_CYCLIC). Addresses pointing to a device's register (e.g. a FIFO)
  are typically fixed.
  
+Per descriptor metadata support

+---
+Some data movement architecure (DMA controller and peripherals) uses metadata


architecure = architecture?


+associated with a transaction. The DMA controller role is to transfer the
+payload and the metadata alongside.
+The metadata itself is not used by the DMA engine itself, but it contains
+parameters, keys, vectors, etc for peripheral or from the peripheral.
+
+The DMAengine framework provides a generic ways to facilitate the metadata for
+descriptors. Depending on the architecture the DMA driver can implement either
+or both of the methods and it is up to the client driver to choose which one
+to use.
+
+- DESC_METADATA_CLIENT
+
+  The metadata buffer is allocated/provided

Re: [PATCH v3 02/14] soc: ti: k3: add navss ringacc driver

2019-10-09 Thread Tero Kristo

On 01/10/2019 09:16, Peter Ujfalusi wrote:

From: Grygorii Strashko 

The Ring Accelerator (RINGACC or RA) provides hardware acceleration to
enable straightforward passing of work between a producer and a consumer.
There is one RINGACC module per NAVSS on TI AM65x SoCs.

The RINGACC converts constant-address read and write accesses to equivalent
read or write accesses to a circular data structure in memory. The RINGACC
eliminates the need for each DMA controller which needs to access ring
elements from having to know the current state of the ring (base address,
current offset). The DMA controller performs a read or write access to a
specific address range (which maps to the source interface on the RINGACC)
and the RINGACC replaces the address for the transaction with a new address
which corresponds to the head or tail element of the ring (head for reads,
tail for writes). Since the RINGACC maintains the state, multiple DMA
controllers or channels are allowed to coherently share the same rings as
applicable. The RINGACC is able to place data which is destined towards
software into cached memory directly.

Supported ring modes:
- Ring Mode
- Messaging Mode
- Credentials Mode
- Queue Manager Mode

TI-SCI integration:

Texas Instrument's System Control Interface (TI-SCI) Message Protocol now
has control over Ringacc module resources management (RM) and Rings
configuration.

The corresponding support of TI-SCI Ringacc module RM protocol
introduced as option through DT parameters:
- ti,sci: phandle on TI-SCI firmware controller DT node
- ti,sci-dev-id: TI-SCI device identifier as per TI-SCI firmware spec

if both parameters present - Ringacc driver will configure/free/reset Rings
using TI-SCI Message Ringacc RM Protocol.

The Ringacc driver manages Rings allocation by itself now and requests
TI-SCI firmware to allocate and configure specific Rings only. It's done
this way because, Linux driver implements two stage Rings allocation and
configuration (allocate ring and configure ring) while I-SCI Message


You missed fixing the typo above: I-SCI to TI-SCI. However, it is just 
cosmetic so I don't mind.


Reviewed-by: Tero Kristo 
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


Re: Lay common foundation to make PVR/SGX work without hacks on OMAP34xx, OMAP36xx, AM335x and potentially OMAP4, OMAP5

2019-10-08 Thread Tero Kristo

On 07/10/2019 22:24, H. Nikolaus Schaller wrote:

Hi Tero,


Am 07.10.2019 um 21:18 schrieb Tero Kristo :

On 07/10/2019 18:52, Tony Lindgren wrote:

Hi,
* H. Nikolaus Schaller  [191005 16:59]:
Please try with Tero's current github branch at github.com/t-kristo/linux-pm.git
5.4-rc1-ipc from few days ago, the earlier versions had still issues.


Yeah, this one should be fixed now.


Ok! Will try asap.




* OMAP5 (Pyra): fails to enable the clocks (did work with the previous version)
[  304.140363] clock-controller:clk::0: failed to enable
[  304.147388] PVR_K:(Error): EnableSGXClocks: pm_runtime_get_sync failed (16)

Hmm no idea what might be up with this one. Did some clkctrl clock
fixes maybe cause a regression here? Tero do you have any ideas?


So, this one I am not too sure, I haven't looked at omap5 graphics clocking. I 
don't think it has anything to do with reset handling though.

Is there some simple way to try this out on board; without PVR module that is?


Yes, I have also seen it when just running the commands in the original commit 
message [1]:

# echo on > $(find /sys -name control | grep \/5600)
# rwmem 0x5600fe00  # OCP Revision
0x5600fe00 = 0x4000
# echo auto > $(find /sys -name control | grep \/5600)
# rwmem 0x5600fe10
# rwmem 0x5624

But I have not yet tested with 5.4-rc2, just 5.4-rc1.


Ok, there is a one liner DTS data fix for this issue, attached.

-Tero

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki>From 57f9fecb167c0ef9f1ae2a1aa93a05ca8add52a2 Mon Sep 17 00:00:00 2001
From: Tero Kristo 
Date: Tue, 8 Oct 2019 10:56:42 +0300
Subject: [PATCH 1/1] ARM: dts: omap5: fix gpu_cm clock provider name

The clkctrl code searches for the parent clockdomain based on the name
of the CM provider node. The introduction of SGX node for omap5 made
the node name for the gpu_cm to be clock-controller. There is no
clockdomain named like this, so the lookup fails. Fix by changing
the node name properly.

Fixes: 394534cb07d8 ("ARM: dts: Configure sgx for omap5")
Signed-off-by: Tero Kristo 
---
 arch/arm/boot/dts/omap54xx-clocks.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/omap54xx-clocks.dtsi b/arch/arm/boot/dts/omap54xx-clocks.dtsi
index 939ec7dfc366..db9885103099 100644
--- a/arch/arm/boot/dts/omap54xx-clocks.dtsi
+++ b/arch/arm/boot/dts/omap54xx-clocks.dtsi
@@ -1160,7 +1160,7 @@
 		};
 	};
 
-	gpu_cm: clock-controller@1500 {
+	gpu_cm: gpu_cm@1500 {
 		compatible = "ti,omap4-cm";
 		reg = <0x1500 0x100>;
 		#address-cells = <1>;
-- 
2.17.1



Re: Lay common foundation to make PVR/SGX work without hacks on OMAP34xx, OMAP36xx, AM335x and potentially OMAP4, OMAP5

2019-10-07 Thread Tero Kristo

On 07/10/2019 18:52, Tony Lindgren wrote:

Hi,

* H. Nikolaus Schaller  [191005 16:59]:

Hi all,
with the arrival of v5.4-rc1 some of Tony's sysc patches have arrived
upstream, so we do no longer need them here.

Therefore, I have rebased my drivers/staging/pvr driver [1] and fixed some
more issues:
* omap4 build only needs to distinguish between omap4420/30/60 and omap4470,
   because the latter has an sgx544 inside and the other sgx540
   This is solved by creating a new omap4470.dts
* I have added proper reg values and interrupts to the omap4 device
   tree node of the sgx (child node of the target-module)
* some updates to my sgxdump and sgxdemo scripts (assuming simple
   Debian Stretch rootfs)
* James Hilliard has contributed a fix for osfunc.c
* omap2plus also needs to be configured for STAGING and PREEMPT
   to be able to compile the driver
* I have added the __always_inline fix [2] which is needed for v5.4 with
   CONFIG_CC_OPTIMIZE_FOR_SIZE=y (which I are enabled on the Letux builds)

Unfortunately Tero's rstctrl patches did not yet make it upstream (or even
linux-next) so I also have a copy in this branch.

Results of first testing are:
* OMAP3530 (OpenPandora, BeagleBoard C): fails with
[  559.247558] PVR_K:(Error): SysLocateDevices: platform_get_resource failed

* DM3730 (GTA04, BeagleBoard XM): kernel module loads

* OMAP4460 (Pandaboard ES): kernel module loads

* AM335x (BeagleBoneBlack): reports a problem with omap_reset_deassert:
[  204.246706] omap_reset_deassert: timedout waiting for gfx:0


Please try with Tero's current github branch at github.com/t-kristo/linux-pm.git
5.4-rc1-ipc from few days ago, the earlier versions had still issues.


Yeah, this one should be fixed now.




* OMAP5 (Pyra): fails to enable the clocks (did work with the previous version)
[  304.140363] clock-controller:clk::0: failed to enable
[  304.147388] PVR_K:(Error): EnableSGXClocks: pm_runtime_get_sync failed (16)


Hmm no idea what might be up with this one. Did some clkctrl clock
fixes maybe cause a regression here? Tero do you have any ideas?


So, this one I am not too sure, I haven't looked at omap5 graphics 
clocking. I don't think it has anything to do with reset handling though.


Is there some simple way to try this out on board; without PVR module 
that is?


-Tero




* OMAP5 with omap2plus_defconfig:
root@letux:~# echo on > $(find /sys -name control | grep \/5600)
[  213.490926] clock-controller:clk::0: failed to enable
root@letux:~#

* pvrsrvctl --start --no-module:
   reports (where the kernel module loads) that the uKernel does not run

So I have several ideas what the reasons for the problems on the non-omap5
devices could be:
* initial code may have some omap5 specific hack inside
* or has omap5 specific magic constants
* uKernel may "know" on which platform it runs and
   we would need differently patched user-space code
   for each one
* omap5 has a dual core sgx544 while the other
   have single core
* the register address translation is not yet correct and
   this inhibits communicating of the user-space libs
   with the uKernel

Maybe, if someone can point me to a complete and working BeagleBone source
tree (any kernel release) which makes use of 1.14.3699939 SDK, I could compare
code and address setup to find what makes the difference.


Regards,

Tony


[1]: https://github.com/openpvrsgx-devgroup/linux_openpvrsgx/commits/letux-pvr
[2]: https://lkml.org/lkml/2019/10/2/201


--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


  1   2   3   4   5   6   >