Re: [PATCH] tracing/synthetic: Print out u64 values properly
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
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
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
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
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>
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>
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>
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
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
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
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'
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
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
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
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
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'
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'
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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