* Re: [PATCH v4 7/7] firmware: smccc: Add ARCH_SOC_ID support
From: Arnd Bergmann @ 2020-05-20 21:51 UTC (permalink / raw)
To: Sudeep Holla
Cc: Mark Rutland, Lorenzo Pieralisi, Catalin Marinas,
linux-kernel@vger.kernel.org, Steven Price, harb, Will Deacon,
Linux ARM
In-Reply-To: <20200518115546.GB16262@bogus>
On Mon, May 18, 2020 at 1:55 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Mon, May 18, 2020 at 11:30:21AM +0200, Arnd Bergmann wrote:
> > On Mon, May 18, 2020 at 11:12 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > > +static ssize_t
> > > +jep106_cont_bank_code_show(struct device *dev, struct device_attribute *attr,
> > > + char *buf)
> > > +{
> > > + return sprintf(buf, "0x%02x\n", JEP106_BANK_CONT_CODE(soc_id_version));
> > > +}
> > > +
> > > +static DEVICE_ATTR_RO(jep106_cont_bank_code);
> > > +
> > > +static ssize_t
> > > +jep106_identification_code_show(struct device *dev,
> > > + struct device_attribute *attr, char *buf)
> > > +{
> > > + return sprintf(buf, "0x%02x\n", JEP106_ID_CODE(soc_id_version));
> > > +}
> >
> > I think we should try hard to avoid nonstandard attributes for the soc device.
> >
>
> I agree with that in general but this is bit different for below mentioned
> reason.
>
> > Did you run into a problem with finding one of the existing attributes
> > that can be used to hold the fields?
> >
>
> Not really! The 2 JEP106 codes can be used to derive the manufacturer which
> could match one of the existing attributes. However doing so might require
> importing the huge JEP106 list as it needs to be maintained and updated
> in the kernel. Also that approach will have the compatibility issue and
> that is the reason for introducing these attributes representing raw
> values for userspace.
I was thinking they codes could just be part of the normal strings rather
than get translated. Can you give an example what they would look like
with your current code?
If you think they should be standard attributes, how about adding them
to the default list, and hardcoding them in the other soc device drivers
based on the information we have available there?
Arnd
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v4 0/7] firmware: smccc: Add basic SMCCC v1.2 + ARCH_SOC_ID support
From: Arnd Bergmann @ 2020-05-20 21:54 UTC (permalink / raw)
To: Will Deacon
Cc: Mark Rutland, Lorenzo Pieralisi, Catalin Marinas,
linux-kernel@vger.kernel.org, Steven Price, harb, Sudeep Holla,
Linux ARM
In-Reply-To: <158999823818.135150.13263761266508812198.b4-ty@kernel.org>
On Wed, May 20, 2020 at 11:29 PM Will Deacon <will@kernel.org> wrote:
>
> On Mon, 18 May 2020 10:12:15 +0100, Sudeep Holla wrote:
> > This patch series adds support for SMCCCv1.2 ARCH_SOC_ID.
> > This doesn't add other changes added in SMCCC v1.2 yet. They will
> > follow these soon along with its first user SPCI/PSA-FF.
> >
> > This is tested using upstream TF-A + the patch[3] fixing the original
> > implementation there.
> >
> > [...]
>
> Applied to arm64 (for-next/smccc), thanks!
>
> [1/7] firmware: smccc: Add HAVE_ARM_SMCCC_DISCOVERY to identify SMCCC v1.1 and above
> https://git.kernel.org/arm64/c/e5bfb21d98b6
> [2/7] firmware: smccc: Update link to latest SMCCC specification
> https://git.kernel.org/arm64/c/15c704ab6244
> [3/7] firmware: smccc: Add the definition for SMCCCv1.2 version/error codes
> https://git.kernel.org/arm64/c/0441bfe7f00a
> [4/7] firmware: smccc: Drop smccc_version enum and use ARM_SMCCC_VERSION_1_x instead
> https://git.kernel.org/arm64/c/ad5a57dfe434
> [5/7] firmware: smccc: Refactor SMCCC specific bits into separate file
> https://git.kernel.org/arm64/c/f2ae97062a48
> [6/7] firmware: smccc: Add function to fetch SMCCC version
> https://git.kernel.org/arm64/c/a4fb17465182
> [7/7] firmware: smccc: Add ARCH_SOC_ID support
> https://git.kernel.org/arm64/c/ce6488f0ce09
>
> Arnd -- Sudeep's reply to you about the sysfs groups seemed reasonable to me,
> but please shout if you'd rather I dropped this in order to pursue an
> alternative approach.
I missed the reply earlier, thanks for pointing me to it again.
I'm not entirely convinced, but don't revert it for now because of that,
I assume we can find a solution.
However, please have a look at the build failure report for patch 5
and fix it if you can see what went wrong.
Arnd
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v4 03/14] PCI: cadence: Add support to use custom read and write accessors
From: Rob Herring @ 2020-05-20 22:07 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: devicetree, Lorenzo Pieralisi, Arnd Bergmann, Greg Kroah-Hartman,
linux-kernel, Tom Joseph, linux-pci, Bjorn Helgaas, linux-omap,
linux-arm-kernel
In-Reply-To: <20200506151429.12255-4-kishon@ti.com>
On Wed, May 06, 2020 at 08:44:18PM +0530, Kishon Vijay Abraham I wrote:
> Add support to use custom read and write accessors. Platforms that
> don't support half word or byte access or any other constraint
> while accessing registers can use this feature to populate custom
> read and write accessors. These custom accessors are used for both
> standard register access and configuration space register access of
> the PCIe host bridge.
>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> drivers/pci/controller/cadence/pcie-cadence.h | 107 +++++++++++++++---
> 1 file changed, 94 insertions(+), 13 deletions(-)
Actually, take back my R-by...
>
> diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h
> index df14ad002fe9..70b6b25153e8 100644
> --- a/drivers/pci/controller/cadence/pcie-cadence.h
> +++ b/drivers/pci/controller/cadence/pcie-cadence.h
> @@ -223,6 +223,11 @@ enum cdns_pcie_msg_routing {
> MSG_ROUTING_GATHER,
> };
>
> +struct cdns_pcie_ops {
> + u32 (*read)(void __iomem *addr, int size);
> + void (*write)(void __iomem *addr, int size, u32 value);
> +};
> +
> /**
> * struct cdns_pcie - private data for Cadence PCIe controller drivers
> * @reg_base: IO mapped register base
> @@ -239,7 +244,7 @@ struct cdns_pcie {
> int phy_count;
> struct phy **phy;
> struct device_link **link;
> - const struct cdns_pcie_common_ops *ops;
> + const struct cdns_pcie_ops *ops;
> };
>
> /**
> @@ -299,69 +304,145 @@ struct cdns_pcie_ep {
> /* Register access */
> static inline void cdns_pcie_writeb(struct cdns_pcie *pcie, u32 reg, u8 value)
> {
> - writeb(value, pcie->reg_base + reg);
> + void __iomem *addr = pcie->reg_base + reg;
> +
> + if (pcie->ops && pcie->ops->write) {
> + pcie->ops->write(addr, 0x1, value);
> + return;
> + }
> +
> + writeb(value, addr);
> }
>
> static inline void cdns_pcie_writew(struct cdns_pcie *pcie, u32 reg, u16 value)
> {
> - writew(value, pcie->reg_base + reg);
> + void __iomem *addr = pcie->reg_base + reg;
> +
> + if (pcie->ops && pcie->ops->write) {
> + pcie->ops->write(addr, 0x2, value);
> + return;
> + }
> +
> + writew(value, addr);
> }
cdns_pcie_writeb and cdns_pcie_writew are used, so remove them.
>
> static inline void cdns_pcie_writel(struct cdns_pcie *pcie, u32 reg, u32 value)
> {
> - writel(value, pcie->reg_base + reg);
> + void __iomem *addr = pcie->reg_base + reg;
> +
> + if (pcie->ops && pcie->ops->write) {
> + pcie->ops->write(addr, 0x4, value);
> + return;
> + }
> +
> + writel(value, addr);
writel isn't broken for you, so you don't need this either.
> }
>
> static inline u32 cdns_pcie_readl(struct cdns_pcie *pcie, u32 reg)
> {
> - return readl(pcie->reg_base + reg);
> + void __iomem *addr = pcie->reg_base + reg;
> +
> + if (pcie->ops && pcie->ops->read)
> + return pcie->ops->read(addr, 0x4);
> +
> + return readl(addr);
And neither is readl.
> }
>
> /* Root Port register access */
> static inline void cdns_pcie_rp_writeb(struct cdns_pcie *pcie,
> u32 reg, u8 value)
> {
> - writeb(value, pcie->reg_base + CDNS_PCIE_RP_BASE + reg);
> + void __iomem *addr = pcie->reg_base + CDNS_PCIE_RP_BASE + reg;
> +
> + if (pcie->ops && pcie->ops->write) {
> + pcie->ops->write(addr, 0x1, value);
> + return;
> + }
> +
> + writeb(value, addr);
> }
>
> static inline void cdns_pcie_rp_writew(struct cdns_pcie *pcie,
> u32 reg, u16 value)
> {
> - writew(value, pcie->reg_base + CDNS_PCIE_RP_BASE + reg);
> + void __iomem *addr = pcie->reg_base + CDNS_PCIE_RP_BASE + reg;
> +
> + if (pcie->ops && pcie->ops->write) {
> + pcie->ops->write(addr, 0x2, value);
> + return;
> + }
> +
> + writew(value, addr);
You removed 2 out of 3 calls to this. I think I'd just make the root
port writes always be 32-bit. It is all just one time init stuff
anyways.
Either rework the calls to assemble the data into 32-bits or keep these
functions and do the RMW here.
> }
>
> /* Endpoint Function register access */
> static inline void cdns_pcie_ep_fn_writeb(struct cdns_pcie *pcie, u8 fn,
> u32 reg, u8 value)
> {
> - writeb(value, pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg);
> + void __iomem *addr = pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg;
> +
> + if (pcie->ops && pcie->ops->write) {
> + pcie->ops->write(addr, 0x1, value);
> + return;
> + }
> +
> + writeb(value, addr);
Same for these EP functions.
Unless there are places where doing a RMW is fundamentally broken like
in config space (not counting the one time init stuff).
Rob
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v4 13/14] misc: pci_endpoint_test: Add J721E in pci_device_id table
From: Rob Herring @ 2020-05-20 22:12 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: devicetree, Lorenzo Pieralisi, Arnd Bergmann, linux-pci,
linux-kernel, Tom Joseph, Rob Herring, Greg Kroah-Hartman,
Bjorn Helgaas, linux-omap, linux-arm-kernel
In-Reply-To: <20200506151429.12255-14-kishon@ti.com>
On Wed, 6 May 2020 20:44:28 +0530, Kishon Vijay Abraham I wrote:
> Add J721E in pci_device_id table so that pci-epf-test can be used
> for testing PCIe EP in J721E.
>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> drivers/misc/pci_endpoint_test.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
Reviewed-by: Rob Herring <robh@kernel.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v4 14/14] MAINTAINERS: Add Kishon Vijay Abraham I for TI J721E SoC PCIe
From: Rob Herring @ 2020-05-20 22:12 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: devicetree, Lorenzo Pieralisi, Arnd Bergmann, linux-pci,
linux-kernel, Tom Joseph, Rob Herring, Greg Kroah-Hartman,
Bjorn Helgaas, linux-omap, linux-arm-kernel
In-Reply-To: <20200506151429.12255-15-kishon@ti.com>
On Wed, 6 May 2020 20:44:29 +0530, Kishon Vijay Abraham I wrote:
> Add Kishon Vijay Abraham I as MAINTAINER for TI J721E SoC PCIe.
>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> MAINTAINERS | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
Acked-by: Rob Herring <robh@kernel.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] media: cedrus: Add support for VP8 decoding
From: Jernej Škrabec @ 2020-05-20 22:30 UTC (permalink / raw)
To: mripard, paul.kocialkowski, Nicolas Dufresne
Cc: devel, gregkh, linux-kernel, wens, hverkuil-cisco, mchehab,
linux-arm-kernel, linux-media
In-Reply-To: <ee0aa12fdf1655c4e563b8fc9753a5ab5e52f4cf.camel@ndufresne.ca>
Dne sreda, 20. maj 2020 ob 23:43:40 CEST je Nicolas Dufresne napisal(a):
> Le mercredi 20 mai 2020 à 23:01 +0200, Jernej Skrabec a écrit :
> > VP8 in Cedrus shares same engine as H264.
> >
> > Note that it seems necessary to call bitstream parsing functions,
> > to parse frame header, otherwise decoded image is garbage. This is
> > contrary to what is driver supposed to do. However, values are not
> > really used, so this might be acceptable. It's possible that bitstream
>
> Have you verified that all values passed through controls are not used
> ? To remain a stateless driver, there is no requirement for parsed data
> to be used, the only requirement is that the reference are used.
> Otherwise doing parallel decoding of two stream of different stream
> would be broken. Have you verified that parallel decoding is working as
> expected ?
I'm not sure if you understand what I meant. Although userspace app parses
frame header and fills all data in VP8 control, driver parses frame header
again, using HW bitstream parsing functionality in cedrus_read_header().
Without that second header parsing in HW, decoded image is garbage. Note that
cedrus_read_header() discards all parsed values and relies on those provided
in controls.
This parsing doesn't cause any problems with parallel decoding or anything.
It's done during frame decoding job, so it doesn't affect any state. It's just
that we shouldn't need to parse header in driver because all data is already
provided in controls. It seems that Cedrus core was never tested without that
HW frame header parsing. I found out that HEVC and H264 frames can sometimes
also be wrongly decoded if no bitstream parsing function is triggered in HW
before final decoding.
I spend a lot of time trying to avoid that header parsing, but I couldn't find
any way around it.
In another words, Cedrus VPU provides two functionalities - HW bitstream
parsing (to speed up header parsing) and video decoding. One would thought
that video decoding can be used independently, if all data from header is
already known, but it can't be.
Best regards,
Jernej
>
> > parsing functions set some internal VPU state, which is later necessary
> > for proper decoding. Biggest suspect is "VP8 probs update" trigger.
> >
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > ---
> >
> > drivers/staging/media/sunxi/cedrus/Makefile | 3 +-
> > drivers/staging/media/sunxi/cedrus/cedrus.c | 8 +
> > drivers/staging/media/sunxi/cedrus/cedrus.h | 15 +
> > .../staging/media/sunxi/cedrus/cedrus_dec.c | 5 +
> > .../staging/media/sunxi/cedrus/cedrus_hw.c | 1 +
> > .../staging/media/sunxi/cedrus/cedrus_regs.h | 80 ++
> > .../staging/media/sunxi/cedrus/cedrus_video.c | 9 +
> > .../staging/media/sunxi/cedrus/cedrus_vp8.c | 699 ++++++++++++++++++
> > 8 files changed, 819 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/staging/media/sunxi/cedrus/cedrus_vp8.c
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [RESEND PATCH v14 02/11] hwmon: pwm-fan: Use 64-bit division macro
From: Guru Das Srinagesh @ 2020-05-20 22:55 UTC (permalink / raw)
To: linux-pwm, Thierry Reding, Uwe Kleine-König
Cc: linux-arm-kernel, Guru Das Srinagesh, Daniel Thompson,
Arnd Bergmann, David Collins, Stephen Boyd, linux-kernel,
Geert Uytterhoeven, Dan Carpenter, Joe Perches,
Subbaraman Narayanamurthy, Lee Jones, Guenter Roeck
In-Reply-To: <cover.1589330178.git.gurus@codeaurora.org>
Since the PWM framework is switching struct pwm_args.period's datatype
to u64, prepare for this transition by using DIV_ROUND_UP_ULL to handle
a 64-bit dividend.
Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
Acked-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/hwmon/pwm-fan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
index 30b7b3e..17bb642 100644
--- a/drivers/hwmon/pwm-fan.c
+++ b/drivers/hwmon/pwm-fan.c
@@ -447,7 +447,7 @@ static int pwm_fan_resume(struct device *dev)
return 0;
pwm_get_args(ctx->pwm, &pargs);
- duty = DIV_ROUND_UP(ctx->pwm_value * (pargs.period - 1), MAX_PWM);
+ duty = DIV_ROUND_UP_ULL(ctx->pwm_value * (pargs.period - 1), MAX_PWM);
ret = pwm_config(ctx->pwm, duty, pargs.period);
if (ret)
return ret;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [RESEND PATCH v14 03/11] ir-rx51: Use 64-bit division macro
From: Guru Das Srinagesh @ 2020-05-20 22:55 UTC (permalink / raw)
To: linux-pwm, Thierry Reding, Uwe Kleine-König
Cc: linux-arm-kernel, Guru Das Srinagesh, Daniel Thompson,
Arnd Bergmann, David Collins, Stephen Boyd, linux-kernel,
Geert Uytterhoeven, Dan Carpenter, Joe Perches,
Subbaraman Narayanamurthy, Lee Jones, Guenter Roeck
In-Reply-To: <cover.1589330178.git.gurus@codeaurora.org>
Since the PWM framework is switching struct pwm_state.period's datatype
to u64, prepare for this transition by using DIV_ROUND_CLOSEST_ULL to
handle a 64-bit dividend.
Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
Acked-by: Sean Young <sean@mess.org>
---
drivers/media/rc/ir-rx51.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 8574eda..9a5dfd7 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -241,7 +241,8 @@ static int ir_rx51_probe(struct platform_device *dev)
}
/* Use default, in case userspace does not set the carrier */
- ir_rx51.freq = DIV_ROUND_CLOSEST(pwm_get_period(pwm), NSEC_PER_SEC);
+ ir_rx51.freq = DIV_ROUND_CLOSEST_ULL(pwm_get_period(pwm),
+ NSEC_PER_SEC);
pwm_put(pwm);
hrtimer_init(&ir_rx51.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [RESEND PATCH v14 05/11] pwm: pwm-imx-tpm: Use 64-bit division macro
From: Guru Das Srinagesh @ 2020-05-20 22:55 UTC (permalink / raw)
To: linux-pwm, Thierry Reding, Uwe Kleine-König
Cc: linux-arm-kernel, Guru Das Srinagesh, Daniel Thompson,
Fabio Estevam, Arnd Bergmann, David Collins, Stephen Boyd,
Shawn Guo, Sascha Hauer, linux-kernel, Geert Uytterhoeven,
Dan Carpenter, Pengutronix Kernel Team, Joe Perches,
Subbaraman Narayanamurthy, Lee Jones, Guenter Roeck,
NXP Linux Team
In-Reply-To: <cover.1589330178.git.gurus@codeaurora.org>
Since the PWM framework is switching struct pwm_state.period's datatype
to u64, prepare for this transition by using DIV64_U64_ROUND_CLOSEST to
handle a 64-bit divisor.
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
---
drivers/pwm/pwm-imx-tpm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pwm/pwm-imx-tpm.c b/drivers/pwm/pwm-imx-tpm.c
index 5f3d7f7..fcdf6be 100644
--- a/drivers/pwm/pwm-imx-tpm.c
+++ b/drivers/pwm/pwm-imx-tpm.c
@@ -124,7 +124,7 @@ static int pwm_imx_tpm_round_state(struct pwm_chip *chip,
real_state->duty_cycle = state->duty_cycle;
tmp = (u64)p->mod * real_state->duty_cycle;
- p->val = DIV_ROUND_CLOSEST_ULL(tmp, real_state->period);
+ p->val = DIV64_U64_ROUND_CLOSEST(tmp, real_state->period);
real_state->polarity = state->polarity;
real_state->enabled = state->enabled;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [RESEND PATCH v14 09/11] backlight: pwm_bl: Use 64-bit division function
From: Guru Das Srinagesh @ 2020-05-20 22:56 UTC (permalink / raw)
To: linux-pwm, Thierry Reding, Uwe Kleine-König
Cc: linux-arm-kernel, Guru Das Srinagesh, Daniel Thompson,
Arnd Bergmann, David Collins, Stephen Boyd, linux-kernel,
Geert Uytterhoeven, Dan Carpenter, Joe Perches,
Subbaraman Narayanamurthy, Lee Jones, Guenter Roeck
In-Reply-To: <cover.1589330178.git.gurus@codeaurora.org>
Since the PWM framework is switching struct pwm_state.period's datatype
to u64, prepare for this transition by using div_u64 to handle a 64-bit
dividend instead of a straight division operation.
Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
drivers/video/backlight/pwm_bl.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 82b8d75..464baad 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -606,7 +606,8 @@ static int pwm_backlight_probe(struct platform_device *pdev)
pb->scale = data->max_brightness;
}
- pb->lth_brightness = data->lth_brightness * (state.period / pb->scale);
+ pb->lth_brightness = data->lth_brightness * (div_u64(state.period,
+ pb->scale));
props.type = BACKLIGHT_RAW;
props.max_brightness = data->max_brightness;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [RESEND PATCH v14 01/11] drm/i915: Use 64-bit division macro
From: Guru Das Srinagesh @ 2020-05-20 22:55 UTC (permalink / raw)
To: linux-pwm, Thierry Reding, Uwe Kleine-König
Cc: linux-arm-kernel, Guru Das Srinagesh, Daniel Thompson,
Arnd Bergmann, David Collins, Stephen Boyd, linux-kernel,
Geert Uytterhoeven, Dan Carpenter, Joe Perches,
Subbaraman Narayanamurthy, Lee Jones, Guenter Roeck
In-Reply-To: <cover.1589330178.git.gurus@codeaurora.org>
Since the PWM framework is switching struct pwm_state.duty_cycle's
datatype to u64, prepare for this transition by using DIV_ROUND_UP_ULL
to handle a 64-bit dividend.
Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Acked-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/display/intel_panel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
index 276f438..81547a0 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.c
+++ b/drivers/gpu/drm/i915/display/intel_panel.c
@@ -1920,7 +1920,7 @@ static int pwm_setup_backlight(struct intel_connector *connector,
return retval;
}
- level = DIV_ROUND_UP(pwm_get_duty_cycle(panel->backlight.pwm) * 100,
+ level = DIV_ROUND_UP_ULL(pwm_get_duty_cycle(panel->backlight.pwm) * 100,
CRC_PMIC_PWM_PERIOD_NS);
panel->backlight.level =
intel_panel_compute_brightness(connector, level);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [RESEND PATCH v14 00/11] Convert PWM period and duty cycle to u64
From: Guru Das Srinagesh @ 2020-05-20 22:55 UTC (permalink / raw)
To: linux-pwm, Thierry Reding, Uwe Kleine-König
Cc: linux-arm-kernel, Guru Das Srinagesh, Daniel Thompson,
Arnd Bergmann, David Collins, Stephen Boyd, linux-kernel,
Geert Uytterhoeven, Dan Carpenter, Joe Perches,
Subbaraman Narayanamurthy, Lee Jones, Guenter Roeck
[REQUEST]
As the patch series stands currently, only 4 out of 11 patches remain
unreviewed. All the other patches have either an Acked-by or a Reviewed-by tag.
Could these remaining patches in the series be reviewed as well?
- 04/11
- 05/11
- 06/11
- 11/11
[COVER LETTER]
Because period and duty cycle are defined in the PWM framework structs as ints
with units of nanoseconds, the maximum time duration that can be set is limited
to ~2.147 seconds. Consequently, applications desiring to set greater time
periods via the PWM framework are not be able to do so - like, for instance,
causing an LED to blink at an interval of 5 seconds.
Redefining the period and duty cycle struct members in the core PWM framework
structs as u64 values will enable larger time durations to be set and solve
this problem. Such a change to the framework mandates that drivers using these
struct members (and corresponding helper functions) also be modified correctly
in order to prevent compilation errors.
This patch series introduces the changes to all the drivers first, followed by
the framework change at the very end so that when the latter is applied, all
the drivers are in good shape and there are no compilation errors.
Changes from v13:
- Pruned cc-list and added same (reduced) set of reviewers to all patches.
- Added Lee Jones' Acked-by to the pwm_bl.c patch.
- Added Jani Nikula's Acked-by to intel-panel.c patch.
- Added Stephen Boyd's Acked-by to pwm-clk.c patch.
- Addressed Geert's review comments in clps711x.c patch.
Changes from v12:
- Rebased to tip of for-next
- Collected Acked-by for sun4i
- Reworked patch for intel-panel.c due to rebase, dropped Jani's Acked-by as
a result
Changes from v11:
- Rebased to tip of for-next.
- Collected "Acked-by:" for v7 (unchanged) of pwm: sifive: [4]
- Squished stm32-lp.c change with final patch in series
- sun4i: Used nsecs_to_jiffies()
- imx27: Added overflow handling logic
- clps711x: Corrected the if condition for skipping the division
- clk: pwm: Reverted to v8 version, added check to prevent division-by-zero
Changes from v10:
- Carefully added back all the "Reviewed-by: " and "Acked-by: " tags received
so far that had gotten missed in v9. No other changes.
Changes from v9:
- Gathered the received "Reviewed-by: " tag
- Added back the clk-pwm.c patch because kbuild test robot complained [3]
and addressed received review comments.
- clps711x: Addressed review comments.
Changes from v8:
- Gathered all received "Acked-by: " and "Reviewed-by: " tags
- Dropped patch to clk-pwm.c for reasons mentiond in [2]
- Expanded audience of unreviewed patches
Changes from v7:
- Changed commit messages of all patches to be brief and to the point.
- Added explanation of change in cover letter.
- Dropped change to pwm-sti.c as upon review it was unnecessary as struct
pwm_capture is not being modified in the PWM core.
Changes from v6:
- Split out the driver changes out into separate patches, one patch per file
for ease of reviewing.
Changes from v5:
- Dropped the conversion of struct pwm_capture to u64 for reasons mentioned
in https://www.spinics.net/lists/linux-pwm/msg11541.html
Changes from v4:
- Split the patch into two: one for changes to the drivers, and the actual
switch to u64 for ease of reverting should the need arise.
- Re-examined the patch and made the following corrections:
* intel_panel.c:
DIV64_U64_ROUND_UP -> DIV_ROUND_UP_ULL (as only the numerator would be
64-bit in this case).
* pwm-sti.c:
do_div -> div_u64 (do_div is optimized only for x86 architectures, and
div_u64's comment block suggests to use this as much as possible).
Changes from v3:
- Rebased to current tip of for-next.
Changes from v2:
- Fixed %u -> %llu in a dev_dbg in pwm-stm32-lp.c, thanks to kbuild test robot
- Added a couple of fixes to pwm-imx-tpm.c and pwm-sifive.c
Changes from v1:
- Fixed compilation errors seen when compiling for different archs.
v1:
- Reworked the change pushed upstream earlier [1] so as to not add an
extension to an obsolete API. With this change, pwm_ops->apply() can be
used to set pwm_state parameters as usual.
[1] https://lore.kernel.org/lkml/20190916140048.GB7488@ulmo/
[2] https://lore.kernel.org/lkml/20200312190859.GA19605@xxxxxxxxxxxxxx/
[3] https://www.spinics.net/lists/linux-pwm/msg11906.html
[4] https://www.spinics.net/lists/linux-pwm/msg11986.html
Guru Das Srinagesh (11):
drm/i915: Use 64-bit division macro
hwmon: pwm-fan: Use 64-bit division macro
ir-rx51: Use 64-bit division macro
pwm: clps711x: Cast period to u32 before use as divisor
pwm: pwm-imx-tpm: Use 64-bit division macro
pwm: imx27: Use 64-bit division macro and function
pwm: sifive: Use 64-bit division macro
pwm: sun4i: Use nsecs_to_jiffies to avoid a division
backlight: pwm_bl: Use 64-bit division function
clk: pwm: Use 64-bit division function
pwm: core: Convert period and duty cycle to u64
drivers/clk/clk-pwm.c | 7 +++-
drivers/gpu/drm/i915/display/intel_panel.c | 2 +-
drivers/hwmon/pwm-fan.c | 2 +-
drivers/media/rc/ir-rx51.c | 3 +-
drivers/pwm/core.c | 14 ++++----
drivers/pwm/pwm-clps711x.c | 5 ++-
drivers/pwm/pwm-imx-tpm.c | 2 +-
drivers/pwm/pwm-imx27.c | 53 +++++++++++++++++++++++++-----
drivers/pwm/pwm-sifive.c | 2 +-
drivers/pwm/pwm-stm32-lp.c | 2 +-
drivers/pwm/pwm-sun4i.c | 2 +-
drivers/pwm/sysfs.c | 8 ++---
drivers/video/backlight/pwm_bl.c | 3 +-
include/linux/pwm.h | 12 +++----
14 files changed, 82 insertions(+), 35 deletions(-)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [RESEND PATCH v14 08/11] pwm: sun4i: Use nsecs_to_jiffies to avoid a division
From: Guru Das Srinagesh @ 2020-05-20 22:56 UTC (permalink / raw)
To: linux-pwm, Thierry Reding, Uwe Kleine-König
Cc: linux-arm-kernel, Guru Das Srinagesh, Daniel Thompson,
Arnd Bergmann, David Collins, Stephen Boyd, linux-kernel,
Geert Uytterhoeven, Dan Carpenter, Joe Perches,
Subbaraman Narayanamurthy, Lee Jones, Guenter Roeck
In-Reply-To: <cover.1589330178.git.gurus@codeaurora.org>
Since the PWM framework is switching struct pwm_state.period's datatype
to u64, prepare for this transition by using nsecs_to_jiffies() which
does away with the need for a division operation.
Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
Acked-by: Chen-Yu Tsai <wens@csie.org>
---
drivers/pwm/pwm-sun4i.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
index 5c677c5..1694e69 100644
--- a/drivers/pwm/pwm-sun4i.c
+++ b/drivers/pwm/pwm-sun4i.c
@@ -285,7 +285,7 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
val = (duty & PWM_DTY_MASK) | PWM_PRD(period);
sun4i_pwm_writel(sun4i_pwm, val, PWM_CH_PRD(pwm->hwpwm));
sun4i_pwm->next_period[pwm->hwpwm] = jiffies +
- usecs_to_jiffies(cstate.period / 1000 + 1);
+ nsecs_to_jiffies(cstate.period + 1000);
if (state->polarity != PWM_POLARITY_NORMAL)
ctrl &= ~BIT_CH(PWM_ACT_STATE, pwm->hwpwm);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [RESEND PATCH v14 07/11] pwm: sifive: Use 64-bit division macro
From: Guru Das Srinagesh @ 2020-05-20 22:56 UTC (permalink / raw)
To: linux-pwm, Thierry Reding, Uwe Kleine-König
Cc: linux-arm-kernel, Guru Das Srinagesh, Daniel Thompson,
Arnd Bergmann, David Collins, Stephen Boyd, linux-kernel,
Geert Uytterhoeven, Dan Carpenter, Joe Perches,
Subbaraman Narayanamurthy, Lee Jones, Guenter Roeck
In-Reply-To: <cover.1589330178.git.gurus@codeaurora.org>
Since the PWM framework is switching struct pwm_args.period's datatype
to u64, prepare for this transition by using DIV64_U64_ROUND_CLOSEST to
handle a 64-bit divisor.
Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>
---
drivers/pwm/pwm-sifive.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c
index cc63f9b..62de0bb 100644
--- a/drivers/pwm/pwm-sifive.c
+++ b/drivers/pwm/pwm-sifive.c
@@ -181,7 +181,7 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm,
* consecutively
*/
num = (u64)duty_cycle * (1U << PWM_SIFIVE_CMPWIDTH);
- frac = DIV_ROUND_CLOSEST_ULL(num, state->period);
+ frac = DIV64_U64_ROUND_CLOSEST(num, state->period);
/* The hardware cannot generate a 100% duty cycle */
frac = min(frac, (1U << PWM_SIFIVE_CMPWIDTH) - 1);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [RESEND PATCH v14 04/11] pwm: clps711x: Cast period to u32 before use as divisor
From: Guru Das Srinagesh @ 2020-05-20 22:55 UTC (permalink / raw)
To: linux-pwm, Thierry Reding, Uwe Kleine-König
Cc: linux-arm-kernel, Guru Das Srinagesh, Daniel Thompson,
Arnd Bergmann, David Collins, Stephen Boyd, linux-kernel,
Geert Uytterhoeven, Dan Carpenter, Joe Perches,
Subbaraman Narayanamurthy, Lee Jones, Guenter Roeck
In-Reply-To: <cover.1589330178.git.gurus@codeaurora.org>
Since the PWM framework is switching struct pwm_args.period's datatype
to u64, prepare for this transition by typecasting it to u32.
Also, since the dividend is still a 32-bit number, any divisor greater
than the numerator will cause the quotient to be zero, so return 0 in
that case to efficiently skip the division.
Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
---
drivers/pwm/pwm-clps711x.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/pwm/pwm-clps711x.c b/drivers/pwm/pwm-clps711x.c
index 924d39a..da771b1 100644
--- a/drivers/pwm/pwm-clps711x.c
+++ b/drivers/pwm/pwm-clps711x.c
@@ -43,7 +43,10 @@ static void clps711x_pwm_update_val(struct clps711x_chip *priv, u32 n, u32 v)
static unsigned int clps711x_get_duty(struct pwm_device *pwm, unsigned int v)
{
/* Duty cycle 0..15 max */
- return DIV_ROUND_CLOSEST(v * 0xf, pwm->args.period);
+ if (pwm->args.period > (v * 0xf))
+ return 0;
+
+ return DIV_ROUND_CLOSEST(v * 0xf, (u32)pwm->args.period);
}
static int clps711x_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [RESEND PATCH v14 06/11] pwm: imx27: Use 64-bit division macro and function
From: Guru Das Srinagesh @ 2020-05-20 22:55 UTC (permalink / raw)
To: linux-pwm, Thierry Reding, Uwe Kleine-König
Cc: linux-arm-kernel, Guru Das Srinagesh, Daniel Thompson,
Fabio Estevam, Arnd Bergmann, David Collins, Stephen Boyd,
Shawn Guo, Sascha Hauer, linux-kernel, Geert Uytterhoeven,
Dan Carpenter, Pengutronix Kernel Team, Joe Perches,
Subbaraman Narayanamurthy, Lee Jones, Guenter Roeck,
NXP Linux Team
In-Reply-To: <cover.1589330178.git.gurus@codeaurora.org>
Since the PWM framework is switching struct pwm_state.period's
datatype to u64, prepare for this transition by using
DIV_ROUND_UP_ULL to handle a 64-bit dividend, and div64_u64 to handle a
64-bit divisor.
Also handle a possible overflow in the calculation of period_cycles when
both clk_rate and period are large numbers. This logic was unit-tested
out by calculating period_cycles using both the existing logic and the
proposed one, and the results are as below.
----------------------------------------------------------------------------
clk_rate period existing proposed
----------------------------------------------------------------------------
1000000000 18446744073709551615 18446744072 18446744073000000000
(U64_MAX)
----------------------------------------------------------------------------
1000000000 4294967291 4294967291 4294967291
----------------------------------------------------------------------------
Overflow occurs in the first case with the existing logic, whereas the
proposed logic handles it better, sacrificing some precision in a best-effort
attempt to handle overflow. As for the second case where there are
more typical values of period, the proposed logic handles that correctly
too.
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
---
drivers/pwm/pwm-imx27.c | 53 +++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 45 insertions(+), 8 deletions(-)
diff --git a/drivers/pwm/pwm-imx27.c b/drivers/pwm/pwm-imx27.c
index a6e40d4..164cb65 100644
--- a/drivers/pwm/pwm-imx27.c
+++ b/drivers/pwm/pwm-imx27.c
@@ -203,7 +203,7 @@ static void pwm_imx27_wait_fifo_slot(struct pwm_chip *chip,
sr = readl(imx->mmio_base + MX3_PWMSR);
fifoav = FIELD_GET(MX3_PWMSR_FIFOAV, sr);
if (fifoav == MX3_PWMSR_FIFOAV_4WORDS) {
- period_ms = DIV_ROUND_UP(pwm_get_period(pwm),
+ period_ms = DIV_ROUND_UP_ULL(pwm_get_period(pwm),
NSEC_PER_MSEC);
msleep(period_ms);
@@ -213,6 +213,45 @@ static void pwm_imx27_wait_fifo_slot(struct pwm_chip *chip,
}
}
+static int pwm_imx27_calc_period_cycles(const struct pwm_state *state,
+ unsigned long clk_rate,
+ unsigned long *period_cycles)
+{
+ u64 c = 0, c1, c2;
+
+ c1 = clk_rate;
+ c2 = state->period;
+ if (c2 > c1) {
+ c2 = c1;
+ c1 = state->period;
+ }
+
+ if (!c1 || !c2) {
+ pr_err("clk rate and period should be nonzero\n");
+ return -EINVAL;
+ }
+
+ if (c2 <= div64_u64(U64_MAX, c1)) {
+ c = c1 * c2;
+ do_div(c, 1000000000);
+ } else if (c2 <= div64_u64(U64_MAX, div64_u64(c1, 1000))) {
+ do_div(c1, 1000);
+ c = c1 * c2;
+ do_div(c, 1000000);
+ } else if (c2 <= div64_u64(U64_MAX, div64_u64(c1, 1000000))) {
+ do_div(c1, 1000000);
+ c = c1 * c2;
+ do_div(c, 1000);
+ } else if (c2 <= div64_u64(U64_MAX, div64_u64(c1, 1000000000))) {
+ do_div(c1, 1000000000);
+ c = c1 * c2;
+ }
+
+ *period_cycles = c;
+
+ return 0;
+}
+
static int pwm_imx27_apply(struct pwm_chip *chip, struct pwm_device *pwm,
const struct pwm_state *state)
{
@@ -225,18 +264,16 @@ static int pwm_imx27_apply(struct pwm_chip *chip, struct pwm_device *pwm,
pwm_get_state(pwm, &cstate);
- c = clk_get_rate(imx->clk_per);
- c *= state->period;
-
- do_div(c, 1000000000);
- period_cycles = c;
+ ret = pwm_imx27_calc_period_cycles(state, clk_get_rate(imx->clk_per),
+ &period_cycles);
+ if (ret)
+ return ret;
prescale = period_cycles / 0x10000 + 1;
period_cycles /= prescale;
c = (unsigned long long)period_cycles * state->duty_cycle;
- do_div(c, state->period);
- duty_cycles = c;
+ duty_cycles = div64_u64(c, state->period);
/*
* according to imx pwm RM, the real period value should be PERIOD
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [RESEND PATCH v14 10/11] clk: pwm: Use 64-bit division function
From: Guru Das Srinagesh @ 2020-05-20 22:56 UTC (permalink / raw)
To: linux-pwm, Thierry Reding, Uwe Kleine-König
Cc: linux-arm-kernel, Guru Das Srinagesh, Daniel Thompson,
Arnd Bergmann, David Collins, Stephen Boyd, linux-kernel,
Geert Uytterhoeven, Dan Carpenter, Joe Perches,
Subbaraman Narayanamurthy, Lee Jones, Guenter Roeck
In-Reply-To: <cover.1589330178.git.gurus@codeaurora.org>
Since the PWM framework is switching struct pwm_args.period's datatype
to u64, prepare for this transition by using div64_u64() to handle a
64-bit divisor.
Also ensure that divide-by-zero (with fixed_rate as denominator) does
not happen with an explicit check with probe failure as a consequence.
Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
Acked-by: Stephen Boyd <sboyd@kernel.org>
---
drivers/clk/clk-pwm.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/clk-pwm.c b/drivers/clk/clk-pwm.c
index 87fe0b0e..86f2e2d 100644
--- a/drivers/clk/clk-pwm.c
+++ b/drivers/clk/clk-pwm.c
@@ -89,7 +89,12 @@ static int clk_pwm_probe(struct platform_device *pdev)
}
if (of_property_read_u32(node, "clock-frequency", &clk_pwm->fixed_rate))
- clk_pwm->fixed_rate = NSEC_PER_SEC / pargs.period;
+ clk_pwm->fixed_rate = div64_u64(NSEC_PER_SEC, pargs.period);
+
+ if (!clk_pwm->fixed_rate) {
+ dev_err(&pdev->dev, "fixed_rate cannot be zero\n");
+ return -EINVAL;
+ }
if (pargs.period != NSEC_PER_SEC / clk_pwm->fixed_rate &&
pargs.period != DIV_ROUND_UP(NSEC_PER_SEC, clk_pwm->fixed_rate)) {
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [RESEND PATCH v14 11/11] pwm: core: Convert period and duty cycle to u64
From: Guru Das Srinagesh @ 2020-05-20 22:56 UTC (permalink / raw)
To: linux-pwm, Thierry Reding, Uwe Kleine-König
Cc: linux-arm-kernel, Guru Das Srinagesh, Daniel Thompson,
Fabio Estevam, Arnd Bergmann, David Collins, Stephen Boyd,
Shawn Guo, Sascha Hauer, linux-kernel, Geert Uytterhoeven,
Dan Carpenter, Pengutronix Kernel Team, Joe Perches,
Subbaraman Narayanamurthy, Lee Jones, Guenter Roeck,
NXP Linux Team
In-Reply-To: <cover.1589330178.git.gurus@codeaurora.org>
Because period and duty cycle are defined as ints with units of
nanoseconds, the maximum time duration that can be set is limited to
~2.147 seconds. Change their definitions to u64 in the structs of the
PWM framework so that higher durations may be set.
Also use the right format specifiers in debug prints in both core.c as
well as pwm-stm32-lp.c.
Reported-by: kbuild test robot <lkp@intel.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
---
drivers/pwm/core.c | 14 +++++++-------
drivers/pwm/pwm-stm32-lp.c | 2 +-
drivers/pwm/sysfs.c | 8 ++++----
include/linux/pwm.h | 12 ++++++------
4 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index bca0496..a2ff6dd 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -510,12 +510,12 @@ static void pwm_apply_state_debug(struct pwm_device *pwm,
last->period > s2.period &&
last->period <= state->period)
dev_warn(chip->dev,
- ".apply didn't pick the best available period (requested: %u, applied: %u, possible: %u)\n",
+ ".apply didn't pick the best available period (requested: %llu, applied: %llu, possible: %llu)\n",
state->period, s2.period, last->period);
if (state->enabled && state->period < s2.period)
dev_warn(chip->dev,
- ".apply is supposed to round down period (requested: %u, applied: %u)\n",
+ ".apply is supposed to round down period (requested: %llu, applied: %llu)\n",
state->period, s2.period);
if (state->enabled &&
@@ -524,14 +524,14 @@ static void pwm_apply_state_debug(struct pwm_device *pwm,
last->duty_cycle > s2.duty_cycle &&
last->duty_cycle <= state->duty_cycle)
dev_warn(chip->dev,
- ".apply didn't pick the best available duty cycle (requested: %u/%u, applied: %u/%u, possible: %u/%u)\n",
+ ".apply didn't pick the best available duty cycle (requested: %llu/%llu, applied: %llu/%llu, possible: %llu/%llu)\n",
state->duty_cycle, state->period,
s2.duty_cycle, s2.period,
last->duty_cycle, last->period);
if (state->enabled && state->duty_cycle < s2.duty_cycle)
dev_warn(chip->dev,
- ".apply is supposed to round down duty_cycle (requested: %u/%u, applied: %u/%u)\n",
+ ".apply is supposed to round down duty_cycle (requested: %llu/%llu, applied: %llu/%llu)\n",
state->duty_cycle, state->period,
s2.duty_cycle, s2.period);
@@ -558,7 +558,7 @@ static void pwm_apply_state_debug(struct pwm_device *pwm,
(s1.enabled && s1.period != last->period) ||
(s1.enabled && s1.duty_cycle != last->duty_cycle)) {
dev_err(chip->dev,
- ".apply is not idempotent (ena=%d pol=%d %u/%u) -> (ena=%d pol=%d %u/%u)\n",
+ ".apply is not idempotent (ena=%d pol=%d %llu/%llu) -> (ena=%d pol=%d %llu/%llu)\n",
s1.enabled, s1.polarity, s1.duty_cycle, s1.period,
last->enabled, last->polarity, last->duty_cycle,
last->period);
@@ -1284,8 +1284,8 @@ static void pwm_dbg_show(struct pwm_chip *chip, struct seq_file *s)
if (state.enabled)
seq_puts(s, " enabled");
- seq_printf(s, " period: %u ns", state.period);
- seq_printf(s, " duty: %u ns", state.duty_cycle);
+ seq_printf(s, " period: %llu ns", state.period);
+ seq_printf(s, " duty: %llu ns", state.duty_cycle);
seq_printf(s, " polarity: %s",
state.polarity ? "inverse" : "normal");
diff --git a/drivers/pwm/pwm-stm32-lp.c b/drivers/pwm/pwm-stm32-lp.c
index 67fca62..134c146 100644
--- a/drivers/pwm/pwm-stm32-lp.c
+++ b/drivers/pwm/pwm-stm32-lp.c
@@ -61,7 +61,7 @@ static int stm32_pwm_lp_apply(struct pwm_chip *chip, struct pwm_device *pwm,
do_div(div, NSEC_PER_SEC);
if (!div) {
/* Clock is too slow to achieve requested period. */
- dev_dbg(priv->chip.dev, "Can't reach %u ns\n", state->period);
+ dev_dbg(priv->chip.dev, "Can't reach %llu ns\n", state->period);
return -EINVAL;
}
diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
index 2389b86..449dbc0 100644
--- a/drivers/pwm/sysfs.c
+++ b/drivers/pwm/sysfs.c
@@ -42,7 +42,7 @@ static ssize_t period_show(struct device *child,
pwm_get_state(pwm, &state);
- return sprintf(buf, "%u\n", state.period);
+ return sprintf(buf, "%llu\n", state.period);
}
static ssize_t period_store(struct device *child,
@@ -52,10 +52,10 @@ static ssize_t period_store(struct device *child,
struct pwm_export *export = child_to_pwm_export(child);
struct pwm_device *pwm = export->pwm;
struct pwm_state state;
- unsigned int val;
+ u64 val;
int ret;
- ret = kstrtouint(buf, 0, &val);
+ ret = kstrtou64(buf, 0, &val);
if (ret)
return ret;
@@ -77,7 +77,7 @@ static ssize_t duty_cycle_show(struct device *child,
pwm_get_state(pwm, &state);
- return sprintf(buf, "%u\n", state.duty_cycle);
+ return sprintf(buf, "%llu\n", state.duty_cycle);
}
static ssize_t duty_cycle_store(struct device *child,
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 2635b2a..a13ff38 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -39,7 +39,7 @@ enum pwm_polarity {
* current PWM hardware state.
*/
struct pwm_args {
- unsigned int period;
+ u64 period;
enum pwm_polarity polarity;
};
@@ -56,8 +56,8 @@ enum {
* @enabled: PWM enabled status
*/
struct pwm_state {
- unsigned int period;
- unsigned int duty_cycle;
+ u64 period;
+ u64 duty_cycle;
enum pwm_polarity polarity;
bool enabled;
};
@@ -107,13 +107,13 @@ static inline bool pwm_is_enabled(const struct pwm_device *pwm)
return state.enabled;
}
-static inline void pwm_set_period(struct pwm_device *pwm, unsigned int period)
+static inline void pwm_set_period(struct pwm_device *pwm, u64 period)
{
if (pwm)
pwm->state.period = period;
}
-static inline unsigned int pwm_get_period(const struct pwm_device *pwm)
+static inline u64 pwm_get_period(const struct pwm_device *pwm)
{
struct pwm_state state;
@@ -128,7 +128,7 @@ static inline void pwm_set_duty_cycle(struct pwm_device *pwm, unsigned int duty)
pwm->state.duty_cycle = duty;
}
-static inline unsigned int pwm_get_duty_cycle(const struct pwm_device *pwm)
+static inline u64 pwm_get_duty_cycle(const struct pwm_device *pwm)
{
struct pwm_state state;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v4 0/5] arm64: dts: meson: add W400 dtsi and GT-King/Pro devices
From: Kevin Hilman @ 2020-05-20 22:57 UTC (permalink / raw)
To: Mark Rutland, Rob Herring, linux-amlogic, linux-arm-kernel,
linux-kernel, devicetree, Christian Hewitt
In-Reply-To: <20200520014329.12469-1-christianshewitt@gmail.com>
On Wed, 20 May 2020 01:43:24 +0000, Christian Hewitt wrote:
> This series combines patch 2 from [1] which converts the existing Ugoos
> AM6 device-tree to a common W400 dtsi and dts, and then reworks the
> Beelink GT-King/GT-King Pro series from [2] to use the dtsi, but this
> time without the offending common audio dtsi approach. I've carried
> forwards acks on bindings from Rob as these did not change.
>
> v4 - rebased against Kevin's v5.8/dt64 branch
>
> [...]
Applied, thanks!
[1/5] arm64: dts: meson: convert ugoos-am6 to common w400 dtsi
commit: 3cb74db9b2561a25701b9024b9d5c0077c43e214
[2/5] dt-bindings: arm: amlogic: add support for the Beelink GT-King
commit: 3a90ef281f852db9900024116e8ea93a49115df9
[3/5] arm64: dts: meson-g12b-gtking: add initial device-tree
commit: c5522ff9c7299f9845df3fd521d51a1ef7617ac7
[4/5] dt-bindings: arm: amlogic: add support for the Beelink GT-King Pro
commit: 8d4b8772296f88e0b6bf5d091ebf25a54e51882c
[5/5] arm64: dts: meson-g12b-gtking-pro: add initial device-tree
commit: 0b928e4e412b1eb9e79e02cf3580b9254d338aae
Best regards,
--
Kevin Hilman <khilman@baylibre.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v4 2/2] PCI: uniphier: Add Socionext UniPhier Pro5 PCIe endpoint controller driver
From: Rob Herring @ 2020-05-20 23:06 UTC (permalink / raw)
To: Kunihiko Hayashi
Cc: devicetree, Lorenzo Pieralisi, Masami Hiramatsu, Jassi Brar,
linux-pci, linux-kernel, Masahiro Yamada, Rob Herring,
Bjorn Helgaas, linux-arm-kernel
In-Reply-To: <1589457801-12796-3-git-send-email-hayashi.kunihiko@socionext.com>
On Thu, 14 May 2020 21:03:21 +0900, Kunihiko Hayashi wrote:
> Add driver for the Socionext UniPhier Pro5 SoC endpoint controller.
> This controller is based on the DesignWare PCIe core.
>
> And add "host" to existing controller descriontions for the host controller
> in Kconfig.
>
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> ---
> MAINTAINERS | 2 +-
> drivers/pci/controller/dwc/Kconfig | 13 +-
> drivers/pci/controller/dwc/Makefile | 1 +
> drivers/pci/controller/dwc/pcie-uniphier-ep.c | 383 ++++++++++++++++++++++++++
> 4 files changed, 396 insertions(+), 3 deletions(-)
> create mode 100644 drivers/pci/controller/dwc/pcie-uniphier-ep.c
>
Reviewed-by: Rob Herring <robh@kernel.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] soc: fsl: qe: Replace one-element array and use struct_size() helper
From: Li Yang @ 2020-05-20 23:52 UTC (permalink / raw)
To: Kees Cook
Cc: Gustavo A. R. Silva, lkml, Gustavo A. R. Silva, linuxppc-dev,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Qiang Zhao
In-Reply-To: <202005181529.C0CB448FBB@keescook>
On Mon, May 18, 2020 at 5:57 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Mon, May 18, 2020 at 05:19:04PM -0500, Gustavo A. R. Silva wrote:
> > The current codebase makes use of one-element arrays in the following
> > form:
> >
> > struct something {
> > int length;
> > u8 data[1];
> > };
> >
> > struct something *instance;
> >
> > instance = kmalloc(sizeof(*instance) + size, GFP_KERNEL);
> > instance->length = size;
> > memcpy(instance->data, source, size);
> >
> > but the preferred mechanism to declare variable-length types such as
> > these ones is a flexible array member[1][2], introduced in C99:
> >
> > struct foo {
> > int stuff;
> > struct boo array[];
> > };
> >
> > By making use of the mechanism above, we will get a compiler warning
> > in case the flexible array does not occur last in the structure, which
> > will help us prevent some kind of undefined behavior bugs from being
> > inadvertently introduced[3] to the codebase from now on. So, replace
> > the one-element array with a flexible-array member.
> >
> > Also, make use of the new struct_size() helper to properly calculate the
> > size of struct qe_firmware.
> >
> > This issue was found with the help of Coccinelle and, audited and fixed
> > _manually_.
> >
> > [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> > [2] https://github.com/KSPP/linux/issues/21
> > [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
> >
> > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> > ---
> > drivers/soc/fsl/qe/qe.c | 4 ++--
> > include/soc/fsl/qe/qe.h | 2 +-
> > 2 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c
> > index 447146861c2c1..2df20d6f85fa4 100644
> > --- a/drivers/soc/fsl/qe/qe.c
> > +++ b/drivers/soc/fsl/qe/qe.c
> > @@ -448,7 +448,7 @@ int qe_upload_firmware(const struct qe_firmware *firmware)
> > unsigned int i;
> > unsigned int j;
> > u32 crc;
> > - size_t calc_size = sizeof(struct qe_firmware);
> > + size_t calc_size;
> > size_t length;
> > const struct qe_header *hdr;
> >
> > @@ -480,7 +480,7 @@ int qe_upload_firmware(const struct qe_firmware *firmware)
> > }
> >
> > /* Validate the length and check if there's a CRC */
> > - calc_size += (firmware->count - 1) * sizeof(struct qe_microcode);
> > + calc_size = struct_size(firmware, microcode, firmware->count);
> >
> > for (i = 0; i < firmware->count; i++)
> > /*
> > diff --git a/include/soc/fsl/qe/qe.h b/include/soc/fsl/qe/qe.h
> > index e282ac01ec081..3feddfec9f87d 100644
> > --- a/include/soc/fsl/qe/qe.h
> > +++ b/include/soc/fsl/qe/qe.h
> > @@ -307,7 +307,7 @@ struct qe_firmware {
> > u8 revision; /* The microcode version revision */
> > u8 padding; /* Reserved, for alignment */
> > u8 reserved[4]; /* Reserved, for future expansion */
> > - } __attribute__ ((packed)) microcode[1];
> > + } __packed microcode[];
> > /* All microcode binaries should be located here */
> > /* CRC32 should be located here, after the microcode binaries */
> > } __attribute__ ((packed));
> > --
> > 2.26.2
> >
>
> Hm, looking at this code, I see a few other things that need to be
> fixed:
>
> 1) drivers/tty/serial/ucc_uart.c does not do a be32_to_cpu() conversion
> on the length test (understandably, a little-endian system has never run
> this code since it's ppc specific), but it's still wrong:
>
> if (firmware->header.length != fw->size) {
>
> compare to the firmware loader:
>
> length = be32_to_cpu(hdr->length);
>
> 2) drivers/soc/fsl/qe/qe.c does not perform bounds checking on the
> per-microcode offsets, so the uploader might send data outside the
> firmware buffer. Perhaps:
We do validate the CRC for each microcode, it is unlikely the CRC
check can pass if the offset or length is not correct. But you are
probably right that it will be safer to check the boundary and fail
quicker before we actually start the CRC check. Will you come up with
a formal patch or you want us to deal with it?
>
>
> diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c
> index 447146861c2c..c4e0bc452f03 100644
> --- a/drivers/soc/fsl/qe/qe.c
> +++ b/drivers/soc/fsl/qe/qe.c
> @@ -451,6 +451,7 @@ int qe_upload_firmware(const struct qe_firmware *firmware)
> size_t calc_size = sizeof(struct qe_firmware);
> size_t length;
> const struct qe_header *hdr;
> + void *firmware_end;
>
> if (!firmware) {
> printk(KERN_ERR "qe-firmware: invalid pointer\n");
> @@ -491,19 +492,39 @@ int qe_upload_firmware(const struct qe_firmware *firmware)
> calc_size += sizeof(__be32) *
> be32_to_cpu(firmware->microcode[i].count);
>
> - /* Validate the length */
> + /* Validate total length */
> if (length != calc_size + sizeof(__be32)) {
> printk(KERN_ERR "qe-firmware: invalid length\n");
> return -EPERM;
> }
>
> /* Validate the CRC */
> - crc = be32_to_cpu(*(__be32 *)((void *)firmware + calc_size));
> + firmware_end = (void *)firmware + calc_size;
> + crc = be32_to_cpu(*(__be32 *)firmware_end);
> if (crc != crc32(0, firmware, calc_size)) {
> printk(KERN_ERR "qe-firmware: firmware CRC is invalid\n");
> return -EIO;
> }
>
> + /* Validate ucode lengths and offsets */
> + for (i = 0; i < firmware->count; i++) {
> + const struct qe_microcode *ucode = &firmware->microcode[i];
> + __be32 *code;
> + size_t count;
> +
> + if (!ucode->code_offset)
> + continue;
> +
> + code = (void *)firmware + be32_to_cpu(ucode->code_offset);
> + count = be32_to_cpu(ucode->count) * sizeof(*code);
> +
> + if (code < firmware || code >= firmware_end ||
> + code + count < firmware || code + count >= firmware_end) {
> + printk(KERN_ERR "qe-firmware: invalid ucode offset\n");
> + return -EIO;
> + }
> + }
> +
> /*
> * If the microcode calls for it, split the I-RAM.
> */
>
>
> I haven't tested this.
>
>
> --
> Kees Cook
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] soc: fsl: qe: Replace one-element array and use struct_size() helper
From: Gustavo A. R. Silva @ 2020-05-21 0:01 UTC (permalink / raw)
To: Li Yang
Cc: Kees Cook, Gustavo A. R. Silva, lkml, linuxppc-dev,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Qiang Zhao
In-Reply-To: <CADRPPNR-Croux9FgnrQJJmdF2jNnuAmC+2xMJSgSbkbRv9u8Mw@mail.gmail.com>
On Wed, May 20, 2020 at 06:52:21PM -0500, Li Yang wrote:
> On Mon, May 18, 2020 at 5:57 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > On Mon, May 18, 2020 at 05:19:04PM -0500, Gustavo A. R. Silva wrote:
> > > The current codebase makes use of one-element arrays in the following
> > > form:
> > >
> > > struct something {
> > > int length;
> > > u8 data[1];
> > > };
> > >
> > > struct something *instance;
> > >
> > > instance = kmalloc(sizeof(*instance) + size, GFP_KERNEL);
> > > instance->length = size;
> > > memcpy(instance->data, source, size);
> > >
> > > but the preferred mechanism to declare variable-length types such as
> > > these ones is a flexible array member[1][2], introduced in C99:
> > >
> > > struct foo {
> > > int stuff;
> > > struct boo array[];
> > > };
> > >
> > > By making use of the mechanism above, we will get a compiler warning
> > > in case the flexible array does not occur last in the structure, which
> > > will help us prevent some kind of undefined behavior bugs from being
> > > inadvertently introduced[3] to the codebase from now on. So, replace
> > > the one-element array with a flexible-array member.
> > >
> > > Also, make use of the new struct_size() helper to properly calculate the
> > > size of struct qe_firmware.
> > >
> > > This issue was found with the help of Coccinelle and, audited and fixed
> > > _manually_.
> > >
> > > [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> > > [2] https://github.com/KSPP/linux/issues/21
> > > [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
> > >
> > > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> > > ---
> > > drivers/soc/fsl/qe/qe.c | 4 ++--
> > > include/soc/fsl/qe/qe.h | 2 +-
> > > 2 files changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c
> > > index 447146861c2c1..2df20d6f85fa4 100644
> > > --- a/drivers/soc/fsl/qe/qe.c
> > > +++ b/drivers/soc/fsl/qe/qe.c
> > > @@ -448,7 +448,7 @@ int qe_upload_firmware(const struct qe_firmware *firmware)
> > > unsigned int i;
> > > unsigned int j;
> > > u32 crc;
> > > - size_t calc_size = sizeof(struct qe_firmware);
> > > + size_t calc_size;
> > > size_t length;
> > > const struct qe_header *hdr;
> > >
> > > @@ -480,7 +480,7 @@ int qe_upload_firmware(const struct qe_firmware *firmware)
> > > }
> > >
> > > /* Validate the length and check if there's a CRC */
> > > - calc_size += (firmware->count - 1) * sizeof(struct qe_microcode);
> > > + calc_size = struct_size(firmware, microcode, firmware->count);
> > >
> > > for (i = 0; i < firmware->count; i++)
> > > /*
> > > diff --git a/include/soc/fsl/qe/qe.h b/include/soc/fsl/qe/qe.h
> > > index e282ac01ec081..3feddfec9f87d 100644
> > > --- a/include/soc/fsl/qe/qe.h
> > > +++ b/include/soc/fsl/qe/qe.h
> > > @@ -307,7 +307,7 @@ struct qe_firmware {
> > > u8 revision; /* The microcode version revision */
> > > u8 padding; /* Reserved, for alignment */
> > > u8 reserved[4]; /* Reserved, for future expansion */
> > > - } __attribute__ ((packed)) microcode[1];
> > > + } __packed microcode[];
> > > /* All microcode binaries should be located here */
> > > /* CRC32 should be located here, after the microcode binaries */
> > > } __attribute__ ((packed));
> > > --
> > > 2.26.2
> > >
> >
> > Hm, looking at this code, I see a few other things that need to be
> > fixed:
> >
> > 1) drivers/tty/serial/ucc_uart.c does not do a be32_to_cpu() conversion
> > on the length test (understandably, a little-endian system has never run
> > this code since it's ppc specific), but it's still wrong:
> >
> > if (firmware->header.length != fw->size) {
> >
> > compare to the firmware loader:
> >
> > length = be32_to_cpu(hdr->length);
> >
> > 2) drivers/soc/fsl/qe/qe.c does not perform bounds checking on the
> > per-microcode offsets, so the uploader might send data outside the
> > firmware buffer. Perhaps:
>
> We do validate the CRC for each microcode, it is unlikely the CRC
> check can pass if the offset or length is not correct. But you are
> probably right that it will be safer to check the boundary and fail
> quicker before we actually start the CRC check. Will you come up with
> a formal patch or you want us to deal with it?
>
Li,
I will send a proper patch for this.
Thanks
--
Gustavo
> >
> >
> > diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c
> > index 447146861c2c..c4e0bc452f03 100644
> > --- a/drivers/soc/fsl/qe/qe.c
> > +++ b/drivers/soc/fsl/qe/qe.c
> > @@ -451,6 +451,7 @@ int qe_upload_firmware(const struct qe_firmware *firmware)
> > size_t calc_size = sizeof(struct qe_firmware);
> > size_t length;
> > const struct qe_header *hdr;
> > + void *firmware_end;
> >
> > if (!firmware) {
> > printk(KERN_ERR "qe-firmware: invalid pointer\n");
> > @@ -491,19 +492,39 @@ int qe_upload_firmware(const struct qe_firmware *firmware)
> > calc_size += sizeof(__be32) *
> > be32_to_cpu(firmware->microcode[i].count);
> >
> > - /* Validate the length */
> > + /* Validate total length */
> > if (length != calc_size + sizeof(__be32)) {
> > printk(KERN_ERR "qe-firmware: invalid length\n");
> > return -EPERM;
> > }
> >
> > /* Validate the CRC */
> > - crc = be32_to_cpu(*(__be32 *)((void *)firmware + calc_size));
> > + firmware_end = (void *)firmware + calc_size;
> > + crc = be32_to_cpu(*(__be32 *)firmware_end);
> > if (crc != crc32(0, firmware, calc_size)) {
> > printk(KERN_ERR "qe-firmware: firmware CRC is invalid\n");
> > return -EIO;
> > }
> >
> > + /* Validate ucode lengths and offsets */
> > + for (i = 0; i < firmware->count; i++) {
> > + const struct qe_microcode *ucode = &firmware->microcode[i];
> > + __be32 *code;
> > + size_t count;
> > +
> > + if (!ucode->code_offset)
> > + continue;
> > +
> > + code = (void *)firmware + be32_to_cpu(ucode->code_offset);
> > + count = be32_to_cpu(ucode->count) * sizeof(*code);
> > +
> > + if (code < firmware || code >= firmware_end ||
> > + code + count < firmware || code + count >= firmware_end) {
> > + printk(KERN_ERR "qe-firmware: invalid ucode offset\n");
> > + return -EIO;
> > + }
> > + }
> > +
> > /*
> > * If the microcode calls for it, split the I-RAM.
> > */
> >
> >
> > I haven't tested this.
> >
> >
> > --
> > Kees Cook
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2 1/4] remoteproc: Introduce rproc_of_parse_firmware() helper
From: Suman Anna @ 2020-05-21 0:10 UTC (permalink / raw)
To: Bjorn Andersson, Rob Herring, Mathieu Poirier
Cc: devicetree, Lokesh Vutla, linux-remoteproc, linux-kernel,
Suman Anna, linux-arm-kernel
In-Reply-To: <20200521001006.2725-1-s-anna@ti.com>
Add a new helper function rproc_of_parse_firmware() to the remoteproc
core that can be used by various remoteproc drivers to look up the
the "firmware-name" property from a rproc device node. This property
is already being used by multiple drivers, so this helper can avoid
repeating equivalent code in remoteproc drivers.
Signed-off-by: Suman Anna <s-anna@ti.com>
---
v2: New patch
drivers/remoteproc/remoteproc_core.c | 23 +++++++++++++++++++++++
drivers/remoteproc/remoteproc_internal.h | 2 ++
2 files changed, 25 insertions(+)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 9f04c30c4aaf..c458b218d524 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1034,6 +1034,29 @@ rproc_of_resm_mem_entry_init(struct device *dev, u32 of_resm_idx, size_t len,
}
EXPORT_SYMBOL(rproc_of_resm_mem_entry_init);
+/**
+ * rproc_of_parse_firmware() - parse and return the firmware-name
+ * @dev: pointer on device struct representing a rproc
+ * @index: index to use for the firmware-name retrieval
+ * @fw_name: pointer to a character string, in which the firmware
+ * name is returned on success and unmodified otherwise.
+ *
+ * This is an OF helper function that parses a device's DT node for
+ * the "firmware-name" property and returns the firmware name pointer
+ * in @fw_name on success.
+ *
+ * Return: 0 on success, or an appropriate failure.
+ */
+int rproc_of_parse_firmware(struct device *dev, int index, const char **fw_name)
+{
+ int ret;
+
+ ret = of_property_read_string_index(dev->of_node, "firmware-name",
+ index, fw_name);
+ return ret ? ret : 0;
+}
+EXPORT_SYMBOL(rproc_of_parse_firmware);
+
/*
* A lookup table for resource handlers. The indices are defined in
* enum fw_resource_type.
diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
index 4ba7cb59d3e8..e5341e91d2fc 100644
--- a/drivers/remoteproc/remoteproc_internal.h
+++ b/drivers/remoteproc/remoteproc_internal.h
@@ -28,6 +28,8 @@ struct rproc_debug_trace {
void rproc_release(struct kref *kref);
irqreturn_t rproc_vq_interrupt(struct rproc *rproc, int vq_id);
void rproc_vdev_release(struct kref *ref);
+int rproc_of_parse_firmware(struct device *dev, int index,
+ const char **fw_name);
/* from remoteproc_virtio.c */
int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id);
--
2.26.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 3/4] remoteproc/k3-dsp: Add a remoteproc driver of K3 C66x DSPs
From: Suman Anna @ 2020-05-21 0:10 UTC (permalink / raw)
To: Bjorn Andersson, Rob Herring, Mathieu Poirier
Cc: devicetree, Lokesh Vutla, linux-remoteproc, linux-kernel,
Suman Anna, linux-arm-kernel
In-Reply-To: <20200521001006.2725-1-s-anna@ti.com>
The Texas Instrument's K3 J721E SoCs have two C66x DSP Subsystems in MAIN
voltage domain that are based on the TI's standard TMS320C66x DSP CorePac
module. Each subsystem has a Fixed/Floating-Point DSP CPU, with 32 KB each
of L1P & L1D SRAMs that can be configured and partitioned as either RAM
and/or Cache, and 288 KB of L2 SRAM with 256 KB of memory configurable as
either RAM and/or Cache. The CorePac also includes an Internal DMA (IDMA),
External Memory Controller (EMC), Extended Memory Controller (XMC) with a
Region Address Translator (RAT) unit for 32-bit to 48-bit address
extension/translations, an Interrupt Controller (INTC) and a Powerdown
Controller (PDC).
A new remoteproc module is added to perform the device management of
these DSP devices. The support is limited to images using only external
DDR memory at the moment, the loading support to internal memories and
any on-chip RAM memories will be added in a subsequent patch. RAT support
is also left for a future patch, and as such the reserved memory carveout
regions are all expected to be using memory regions within the first 2 GB.
Error Recovery and Power Management features are not currently supported.
The C66x remote processors do not have an MMU, and so require fixed memory
carveout regions matching the firmware image addresses. Support for this
is provided by mandating multiple memory regions to be attached to the
remoteproc device. The first memory region will be used to serve as the
DMA pool for all dynamic allocations like the vrings and vring buffers.
The remaining memory regions are mapped into the kernel at device probe
time, and are used to provide address translations for firmware image
segments without the need for any RSC_CARVEOUT entries. Any firmware
image using memory outside of the supplied reserved memory carveout
regions will be errored out.
The driver uses various TI-SCI interfaces to talk to the System Controller
(DMSC) for managing configuration, power and reset management of these
cores. IPC between the A72 cores and the DSP cores is supported through
the virtio rpmsg stack using shared memory and OMAP Mailboxes.
Signed-off-by: Suman Anna <s-anna@ti.com>
---
v2:
- Dropped the pm_runtime usage
- Replaced the private k3_dsp_rproc_get_firmware() with the newly introduced
rproc_of_parse_firmware()
- Addressed other minor comments from Mathieu - Revised help on Kconfig, reordered
header files, whitespace indentation fixes, remove the stale memset comment on
internal memories, renamed struct k3_dsp_rproc_mem to struct k3_dsp_mem.
v1: https://patchwork.kernel.org/patch/11458577/
drivers/remoteproc/Kconfig | 13 +
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/ti_k3_dsp_remoteproc.c | 701 ++++++++++++++++++++++
3 files changed, 715 insertions(+)
create mode 100644 drivers/remoteproc/ti_k3_dsp_remoteproc.c
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 2c9fb870a276..ef787774b52b 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -265,6 +265,19 @@ config TI_K3_R5_REMOTEPROC
It's safe to say N here if you're not interested in utilizing
a slave processor
+config TI_K3_DSP_REMOTEPROC
+ tristate "TI K3 DSP remoteproc support"
+ depends on ARCH_K3
+ select MAILBOX
+ select OMAP2PLUS_MBOX
+ help
+ Say m here to support TI's C66x and C71x DSP remote processor
+ subsystems on various TI K3 family of SoCs through the remote
+ processor framework.
+
+ It's safe to say N here if you're not interested in utilizing
+ the DSP slave processors.
+
endif # REMOTEPROC
endmenu
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index ea0c6812e4fc..9f87d222744c 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -31,3 +31,4 @@ obj-$(CONFIG_ST_REMOTEPROC) += st_remoteproc.o
obj-$(CONFIG_ST_SLIM_REMOTEPROC) += st_slim_rproc.o
obj-$(CONFIG_STM32_RPROC) += stm32_rproc.o
obj-$(CONFIG_TI_K3_R5_REMOTEPROC) += ti_k3_r5_remoteproc.o
+obj-$(CONFIG_TI_K3_DSP_REMOTEPROC) += ti_k3_dsp_remoteproc.o
diff --git a/drivers/remoteproc/ti_k3_dsp_remoteproc.c b/drivers/remoteproc/ti_k3_dsp_remoteproc.c
new file mode 100644
index 000000000000..e4036f5992fe
--- /dev/null
+++ b/drivers/remoteproc/ti_k3_dsp_remoteproc.c
@@ -0,0 +1,701 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * TI K3 DSP Remote Processor(s) driver
+ *
+ * Copyright (C) 2018-2020 Texas Instruments Incorporated - http://www.ti.com/
+ * Suman Anna <s-anna@ti.com>
+ */
+
+#include <linux/io.h>
+#include <linux/mailbox_client.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/omap-mailbox.h>
+#include <linux/platform_device.h>
+#include <linux/remoteproc.h>
+#include <linux/reset.h>
+#include <linux/soc/ti/ti_sci_protocol.h>
+
+#include "omap_remoteproc.h"
+#include "remoteproc_internal.h"
+#include "ti_sci_proc.h"
+
+#define KEYSTONE_RPROC_LOCAL_ADDRESS_MASK (SZ_16M - 1)
+
+/**
+ * struct k3_dsp_mem - internal memory structure
+ * @cpu_addr: MPU virtual address of the memory region
+ * @bus_addr: Bus address used to access the memory region
+ * @dev_addr: Device address of the memory region from DSP view
+ * @size: Size of the memory region
+ */
+struct k3_dsp_mem {
+ void __iomem *cpu_addr;
+ phys_addr_t bus_addr;
+ u32 dev_addr;
+ size_t size;
+};
+
+/**
+ * struct k3_dsp_mem_data - memory definitions for a DSP
+ * @name: name for this memory entry
+ * @dev_addr: device address for the memory entry
+ */
+struct k3_dsp_mem_data {
+ const char *name;
+ const u32 dev_addr;
+};
+
+/**
+ * struct k3_dsp_dev_data - device data structure for a DSP
+ * @mems: pointer to memory definitions for a DSP
+ * @num_mems: number of memory regions in @mems
+ * @boot_align_addr: boot vector address alignment granularity
+ * @uses_lreset: flag to denote the need for local reset management
+ */
+struct k3_dsp_dev_data {
+ const struct k3_dsp_mem_data *mems;
+ u32 num_mems;
+ u32 boot_align_addr;
+ bool uses_lreset;
+};
+
+/**
+ * struct k3_dsp_rproc - k3 DSP remote processor driver structure
+ * @dev: cached device pointer
+ * @rproc: remoteproc device handle
+ * @mem: internal memory regions data
+ * @num_mems: number of internal memory regions
+ * @rmem: reserved memory regions data
+ * @num_rmems: number of reserved memory regions
+ * @reset: reset control handle
+ * @data: pointer to DSP-specific device data
+ * @tsp: TI-SCI processor control handle
+ * @ti_sci: TI-SCI handle
+ * @ti_sci_id: TI-SCI device identifier
+ * @mbox: mailbox channel handle
+ * @client: mailbox client to request the mailbox channel
+ */
+struct k3_dsp_rproc {
+ struct device *dev;
+ struct rproc *rproc;
+ struct k3_dsp_mem *mem;
+ int num_mems;
+ struct k3_dsp_mem *rmem;
+ int num_rmems;
+ struct reset_control *reset;
+ const struct k3_dsp_dev_data *data;
+ struct ti_sci_proc *tsp;
+ const struct ti_sci_handle *ti_sci;
+ u32 ti_sci_id;
+ struct mbox_chan *mbox;
+ struct mbox_client client;
+};
+
+/**
+ * k3_dsp_rproc_mbox_callback() - inbound mailbox message handler
+ * @client: mailbox client pointer used for requesting the mailbox channel
+ * @data: mailbox payload
+ *
+ * This handler is invoked by the OMAP mailbox driver whenever a mailbox
+ * message is received. Usually, the mailbox payload simply contains
+ * the index of the virtqueue that is kicked by the remote processor,
+ * and we let remoteproc core handle it.
+ *
+ * In addition to virtqueue indices, we also have some out-of-band values
+ * that indicate different events. Those values are deliberately very
+ * large so they don't coincide with virtqueue indices.
+ */
+static void k3_dsp_rproc_mbox_callback(struct mbox_client *client, void *data)
+{
+ struct k3_dsp_rproc *kproc = container_of(client, struct k3_dsp_rproc,
+ client);
+ struct device *dev = kproc->rproc->dev.parent;
+ const char *name = kproc->rproc->name;
+ u32 msg = omap_mbox_message(data);
+
+ dev_dbg(dev, "mbox msg: 0x%x\n", msg);
+
+ switch (msg) {
+ case RP_MBOX_CRASH:
+ /*
+ * remoteproc detected an exception, but error recovery is not
+ * supported. So, just log this for now
+ */
+ dev_err(dev, "K3 DSP rproc %s crashed\n", name);
+ break;
+ case RP_MBOX_ECHO_REPLY:
+ dev_info(dev, "received echo reply from %s\n", name);
+ break;
+ default:
+ /* silently handle all other valid messages */
+ if (msg >= RP_MBOX_READY && msg < RP_MBOX_END_MSG)
+ return;
+ if (msg > kproc->rproc->max_notifyid) {
+ dev_dbg(dev, "dropping unknown message 0x%x", msg);
+ return;
+ }
+ /* msg contains the index of the triggered vring */
+ if (rproc_vq_interrupt(kproc->rproc, msg) == IRQ_NONE)
+ dev_dbg(dev, "no message was found in vqid %d\n", msg);
+ }
+}
+
+/*
+ * Kick the remote processor to notify about pending unprocessed messages.
+ * The vqid usage is not used and is inconsequential, as the kick is performed
+ * through a simulated GPIO (a bit in an IPC interrupt-triggering register),
+ * the remote processor is expected to process both its Tx and Rx virtqueues.
+ */
+static void k3_dsp_rproc_kick(struct rproc *rproc, int vqid)
+{
+ struct k3_dsp_rproc *kproc = rproc->priv;
+ struct device *dev = rproc->dev.parent;
+ mbox_msg_t msg = (mbox_msg_t)vqid;
+ int ret;
+
+ /* send the index of the triggered virtqueue in the mailbox payload */
+ ret = mbox_send_message(kproc->mbox, (void *)msg);
+ if (ret < 0)
+ dev_err(dev, "failed to send mailbox message, status = %d\n",
+ ret);
+}
+
+/* Put the DSP processor into reset */
+static int k3_dsp_rproc_reset(struct k3_dsp_rproc *kproc)
+{
+ struct device *dev = kproc->dev;
+ int ret;
+
+ ret = reset_control_assert(kproc->reset);
+ if (ret) {
+ dev_err(dev, "local-reset assert failed, ret = %d\n", ret);
+ return ret;
+ }
+
+ ret = kproc->ti_sci->ops.dev_ops.put_device(kproc->ti_sci,
+ kproc->ti_sci_id);
+ if (ret) {
+ dev_err(dev, "module-reset assert failed, ret = %d\n", ret);
+ if (reset_control_deassert(kproc->reset))
+ dev_warn(dev, "local-reset deassert back failed\n");
+ }
+
+ return ret;
+}
+
+/* Release the DSP processor from reset */
+static int k3_dsp_rproc_release(struct k3_dsp_rproc *kproc)
+{
+ struct device *dev = kproc->dev;
+ int ret;
+
+ ret = kproc->ti_sci->ops.dev_ops.get_device(kproc->ti_sci,
+ kproc->ti_sci_id);
+ if (ret) {
+ dev_err(dev, "module-reset deassert failed, ret = %d\n", ret);
+ return ret;
+ }
+
+ ret = reset_control_deassert(kproc->reset);
+ if (ret) {
+ dev_err(dev, "local-reset deassert failed, ret = %d\n", ret);
+ if (kproc->ti_sci->ops.dev_ops.put_device(kproc->ti_sci,
+ kproc->ti_sci_id))
+ dev_warn(dev, "module-reset assert back failed\n");
+ }
+
+ return ret;
+}
+
+/*
+ * Power up the DSP remote processor.
+ *
+ * This function will be invoked only after the firmware for this rproc
+ * was loaded, parsed successfully, and all of its resource requirements
+ * were met.
+ */
+static int k3_dsp_rproc_start(struct rproc *rproc)
+{
+ struct k3_dsp_rproc *kproc = rproc->priv;
+ struct mbox_client *client = &kproc->client;
+ struct device *dev = kproc->dev;
+ u32 boot_addr;
+ int ret;
+
+ client->dev = dev;
+ client->tx_done = NULL;
+ client->rx_callback = k3_dsp_rproc_mbox_callback;
+ client->tx_block = false;
+ client->knows_txdone = false;
+
+ kproc->mbox = mbox_request_channel(client, 0);
+ if (IS_ERR(kproc->mbox)) {
+ ret = -EBUSY;
+ dev_err(dev, "mbox_request_channel failed: %ld\n",
+ PTR_ERR(kproc->mbox));
+ return ret;
+ }
+
+ /*
+ * Ping the remote processor, this is only for sanity-sake for now;
+ * there is no functional effect whatsoever.
+ *
+ * Note that the reply will _not_ arrive immediately: this message
+ * will wait in the mailbox fifo until the remote processor is booted.
+ */
+ ret = mbox_send_message(kproc->mbox, (void *)RP_MBOX_ECHO_REQUEST);
+ if (ret < 0) {
+ dev_err(dev, "mbox_send_message failed: %d\n", ret);
+ goto put_mbox;
+ }
+
+ boot_addr = rproc->bootaddr;
+ if (boot_addr & (kproc->data->boot_align_addr - 1)) {
+ dev_err(dev, "invalid boot address 0x%x, must be aligned on a 0x%x boundary\n",
+ boot_addr, kproc->data->boot_align_addr);
+ ret = -EINVAL;
+ goto put_mbox;
+ }
+
+ dev_err(dev, "booting DSP core using boot addr = 0x%x\n", boot_addr);
+ ret = ti_sci_proc_set_config(kproc->tsp, boot_addr, 0, 0);
+ if (ret)
+ goto put_mbox;
+
+ ret = k3_dsp_rproc_release(kproc);
+ if (ret)
+ goto put_mbox;
+
+ return 0;
+
+put_mbox:
+ mbox_free_channel(kproc->mbox);
+ return ret;
+}
+
+/*
+ * Stop the DSP remote processor.
+ *
+ * This function puts the DSP processor into reset, and finishes processing
+ * of any pending messages.
+ */
+static int k3_dsp_rproc_stop(struct rproc *rproc)
+{
+ struct k3_dsp_rproc *kproc = rproc->priv;
+
+ mbox_free_channel(kproc->mbox);
+
+ k3_dsp_rproc_reset(kproc);
+
+ return 0;
+}
+
+/*
+ * Custom function to translate a DSP device address (internal RAMs only) to a
+ * kernel virtual address. The DSPs can access their RAMs at either an internal
+ * address visible only from a DSP, or at the SoC-level bus address. Both these
+ * addresses need to be looked through for translation. The translated addresses
+ * can be used either by the remoteproc core for loading (when using kernel
+ * remoteproc loader), or by any rpmsg bus drivers.
+ */
+static void *k3_dsp_rproc_da_to_va(struct rproc *rproc, u64 da, size_t len)
+{
+ struct k3_dsp_rproc *kproc = rproc->priv;
+ void __iomem *va = NULL;
+ phys_addr_t bus_addr;
+ u32 dev_addr, offset;
+ size_t size;
+ int i;
+
+ if (len == 0)
+ return NULL;
+
+ for (i = 0; i < kproc->num_mems; i++) {
+ bus_addr = kproc->mem[i].bus_addr;
+ dev_addr = kproc->mem[i].dev_addr;
+ size = kproc->mem[i].size;
+
+ if (da < KEYSTONE_RPROC_LOCAL_ADDRESS_MASK) {
+ /* handle DSP-view addresses */
+ if (da >= dev_addr &&
+ ((da + len) <= (dev_addr + size))) {
+ offset = da - dev_addr;
+ va = kproc->mem[i].cpu_addr + offset;
+ return (__force void *)va;
+ }
+ } else {
+ /* handle SoC-view addresses */
+ if (da >= bus_addr &&
+ (da + len) <= (bus_addr + size)) {
+ offset = da - bus_addr;
+ va = kproc->mem[i].cpu_addr + offset;
+ return (__force void *)va;
+ }
+ }
+ }
+
+ /* handle static DDR reserved memory regions */
+ for (i = 0; i < kproc->num_rmems; i++) {
+ dev_addr = kproc->rmem[i].dev_addr;
+ size = kproc->rmem[i].size;
+
+ if (da >= dev_addr && ((da + len) <= (dev_addr + size))) {
+ offset = da - dev_addr;
+ va = kproc->rmem[i].cpu_addr + offset;
+ return (__force void *)va;
+ }
+ }
+
+ return NULL;
+}
+
+static const struct rproc_ops k3_dsp_rproc_ops = {
+ .start = k3_dsp_rproc_start,
+ .stop = k3_dsp_rproc_stop,
+ .kick = k3_dsp_rproc_kick,
+ .da_to_va = k3_dsp_rproc_da_to_va,
+};
+
+static int k3_dsp_rproc_of_get_memories(struct platform_device *pdev,
+ struct k3_dsp_rproc *kproc)
+{
+ const struct k3_dsp_dev_data *data = kproc->data;
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+ int num_mems = 0;
+ int i;
+
+ num_mems = kproc->data->num_mems;
+ kproc->mem = devm_kcalloc(kproc->dev, num_mems,
+ sizeof(*kproc->mem), GFP_KERNEL);
+ if (!kproc->mem)
+ return -ENOMEM;
+
+ for (i = 0; i < num_mems; i++) {
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+ data->mems[i].name);
+ if (!res) {
+ dev_err(dev, "found no memory resource for %s\n",
+ data->mems[i].name);
+ return -EINVAL;
+ }
+ if (!devm_request_mem_region(dev, res->start,
+ resource_size(res),
+ dev_name(dev))) {
+ dev_err(dev, "could not request %s region for resource\n",
+ data->mems[i].name);
+ return -EBUSY;
+ }
+
+ kproc->mem[i].cpu_addr = devm_ioremap_wc(dev, res->start,
+ resource_size(res));
+ if (IS_ERR(kproc->mem[i].cpu_addr)) {
+ dev_err(dev, "failed to map %s memory\n",
+ data->mems[i].name);
+ return PTR_ERR(kproc->mem[i].cpu_addr);
+ }
+ kproc->mem[i].bus_addr = res->start;
+ kproc->mem[i].dev_addr = data->mems[i].dev_addr;
+ kproc->mem[i].size = resource_size(res);
+
+ dev_dbg(dev, "memory %8s: bus addr %pa size 0x%zx va %pK da 0x%x\n",
+ data->mems[i].name, &kproc->mem[i].bus_addr,
+ kproc->mem[i].size, kproc->mem[i].cpu_addr,
+ kproc->mem[i].dev_addr);
+ }
+ kproc->num_mems = num_mems;
+
+ return 0;
+}
+
+static int k3_dsp_reserved_mem_init(struct k3_dsp_rproc *kproc)
+{
+ struct device *dev = kproc->dev;
+ struct device_node *np = dev->of_node;
+ struct device_node *rmem_np;
+ struct reserved_mem *rmem;
+ int num_rmems;
+ int ret, i;
+
+ num_rmems = of_property_count_elems_of_size(np, "memory-region",
+ sizeof(phandle));
+ if (num_rmems <= 0) {
+ dev_err(dev, "device does not reserved memory regions, ret = %d\n",
+ num_rmems);
+ return -EINVAL;
+ }
+ if (num_rmems < 2) {
+ dev_err(dev, "device needs atleast two memory regions to be defined, num = %d\n",
+ num_rmems);
+ return -EINVAL;
+ }
+
+ /* use reserved memory region 0 for vring DMA allocations */
+ ret = of_reserved_mem_device_init_by_idx(dev, np, 0);
+ if (ret) {
+ dev_err(dev, "device cannot initialize DMA pool, ret = %d\n",
+ ret);
+ return ret;
+ }
+
+ num_rmems--;
+ kproc->rmem = kcalloc(num_rmems, sizeof(*kproc->rmem), GFP_KERNEL);
+ if (!kproc->rmem) {
+ ret = -ENOMEM;
+ goto release_rmem;
+ }
+
+ /* use remaining reserved memory regions for static carveouts */
+ for (i = 0; i < num_rmems; i++) {
+ rmem_np = of_parse_phandle(np, "memory-region", i + 1);
+ if (!rmem_np) {
+ ret = -EINVAL;
+ goto unmap_rmem;
+ }
+
+ rmem = of_reserved_mem_lookup(rmem_np);
+ if (!rmem) {
+ of_node_put(rmem_np);
+ ret = -EINVAL;
+ goto unmap_rmem;
+ }
+ of_node_put(rmem_np);
+
+ kproc->rmem[i].bus_addr = rmem->base;
+ /* 64-bit address regions currently not supported */
+ kproc->rmem[i].dev_addr = (u32)rmem->base;
+ kproc->rmem[i].size = rmem->size;
+ kproc->rmem[i].cpu_addr = ioremap_wc(rmem->base, rmem->size);
+ if (!kproc->rmem[i].cpu_addr) {
+ dev_err(dev, "failed to map reserved memory#%d at %pa of size %pa\n",
+ i + 1, &rmem->base, &rmem->size);
+ ret = -ENOMEM;
+ goto unmap_rmem;
+ }
+
+ dev_dbg(dev, "reserved memory%d: bus addr %pa size 0x%zx va %pK da 0x%x\n",
+ i + 1, &kproc->rmem[i].bus_addr,
+ kproc->rmem[i].size, kproc->rmem[i].cpu_addr,
+ kproc->rmem[i].dev_addr);
+ }
+ kproc->num_rmems = num_rmems;
+
+ return 0;
+
+unmap_rmem:
+ for (i--; i >= 0; i--) {
+ if (kproc->rmem[i].cpu_addr)
+ iounmap(kproc->rmem[i].cpu_addr);
+ }
+ kfree(kproc->rmem);
+release_rmem:
+ of_reserved_mem_device_release(kproc->dev);
+ return ret;
+}
+
+static void k3_dsp_reserved_mem_exit(struct k3_dsp_rproc *kproc)
+{
+ int i;
+
+ for (i = 0; i < kproc->num_rmems; i++)
+ iounmap(kproc->rmem[i].cpu_addr);
+ kfree(kproc->rmem);
+
+ of_reserved_mem_device_release(kproc->dev);
+}
+
+static
+struct ti_sci_proc *k3_dsp_rproc_of_get_tsp(struct device *dev,
+ const struct ti_sci_handle *sci)
+{
+ struct ti_sci_proc *tsp;
+ u32 temp[2];
+ int ret;
+
+ ret = of_property_read_u32_array(dev->of_node, "ti,sci-proc-ids",
+ temp, 2);
+ if (ret < 0)
+ return ERR_PTR(ret);
+
+ tsp = kzalloc(sizeof(*tsp), GFP_KERNEL);
+ if (!tsp)
+ return ERR_PTR(-ENOMEM);
+
+ tsp->dev = dev;
+ tsp->sci = sci;
+ tsp->ops = &sci->ops.proc_ops;
+ tsp->proc_id = temp[0];
+ tsp->host_id = temp[1];
+
+ return tsp;
+}
+
+static int k3_dsp_rproc_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+ const struct k3_dsp_dev_data *data;
+ struct k3_dsp_rproc *kproc;
+ struct rproc *rproc;
+ const char *fw_name;
+ int ret = 0;
+ int ret1;
+
+ data = of_device_get_match_data(dev);
+ if (!data)
+ return -ENODEV;
+
+ ret = rproc_of_parse_firmware(dev, 0, &fw_name);
+ if (ret) {
+ dev_err(dev, "failed to parse firmware-name property, ret = %d\n",
+ ret);
+ return ret;
+ }
+
+ rproc = rproc_alloc(dev, dev_name(dev), &k3_dsp_rproc_ops, fw_name,
+ sizeof(*kproc));
+ if (!rproc)
+ return -ENOMEM;
+
+ rproc->has_iommu = false;
+ rproc->recovery_disabled = true;
+ kproc = rproc->priv;
+ kproc->rproc = rproc;
+ kproc->dev = dev;
+ kproc->data = data;
+
+ kproc->ti_sci = ti_sci_get_by_phandle(np, "ti,sci");
+ if (IS_ERR(kproc->ti_sci)) {
+ ret = PTR_ERR(kproc->ti_sci);
+ if (ret != -EPROBE_DEFER) {
+ dev_err(dev, "failed to get ti-sci handle, ret = %d\n",
+ ret);
+ }
+ kproc->ti_sci = NULL;
+ goto free_rproc;
+ }
+
+ ret = of_property_read_u32(np, "ti,sci-dev-id", &kproc->ti_sci_id);
+ if (ret) {
+ dev_err(dev, "missing 'ti,sci-dev-id' property\n");
+ goto put_sci;
+ }
+
+ kproc->reset = devm_reset_control_get_exclusive(dev, NULL);
+ if (IS_ERR(kproc->reset)) {
+ ret = PTR_ERR(kproc->reset);
+ dev_err(dev, "failed to get reset, status = %d\n", ret);
+ goto put_sci;
+ }
+
+ kproc->tsp = k3_dsp_rproc_of_get_tsp(dev, kproc->ti_sci);
+ if (IS_ERR(kproc->tsp)) {
+ dev_err(dev, "failed to construct ti-sci proc control, ret = %d\n",
+ ret);
+ ret = PTR_ERR(kproc->tsp);
+ goto put_sci;
+ }
+
+ ret = ti_sci_proc_request(kproc->tsp);
+ if (ret < 0) {
+ dev_err(dev, "ti_sci_proc_request failed, ret = %d\n", ret);
+ goto free_tsp;
+ }
+
+ ret = k3_dsp_rproc_of_get_memories(pdev, kproc);
+ if (ret)
+ goto release_tsp;
+
+ ret = k3_dsp_reserved_mem_init(kproc);
+ if (ret) {
+ dev_err(dev, "reserved memory init failed, ret = %d\n", ret);
+ goto release_tsp;
+ }
+
+ ret = rproc_add(rproc);
+ if (ret) {
+ dev_err(dev, "failed to add register device with remoteproc core, status = %d\n",
+ ret);
+ goto release_mem;
+ }
+
+ platform_set_drvdata(pdev, kproc);
+
+ return 0;
+
+release_mem:
+ k3_dsp_reserved_mem_exit(kproc);
+release_tsp:
+ ret1 = ti_sci_proc_release(kproc->tsp);
+ if (ret1)
+ dev_err(dev, "failed to release proc, ret = %d\n", ret1);
+free_tsp:
+ kfree(kproc->tsp);
+put_sci:
+ ret1 = ti_sci_put_handle(kproc->ti_sci);
+ if (ret1)
+ dev_err(dev, "failed to put ti_sci handle, ret = %d\n", ret1);
+free_rproc:
+ rproc_free(rproc);
+ return ret;
+}
+
+static int k3_dsp_rproc_remove(struct platform_device *pdev)
+{
+ struct k3_dsp_rproc *kproc = platform_get_drvdata(pdev);
+ struct device *dev = &pdev->dev;
+ int ret;
+
+ rproc_del(kproc->rproc);
+
+ ret = ti_sci_proc_release(kproc->tsp);
+ if (ret)
+ dev_err(dev, "failed to release proc, ret = %d\n", ret);
+
+ kfree(kproc->tsp);
+
+ ret = ti_sci_put_handle(kproc->ti_sci);
+ if (ret)
+ dev_err(dev, "failed to put ti_sci handle, ret = %d\n", ret);
+
+ k3_dsp_reserved_mem_exit(kproc);
+ rproc_free(kproc->rproc);
+
+ return 0;
+}
+
+static const struct k3_dsp_mem_data c66_mems[] = {
+ { .name = "l2sram", .dev_addr = 0x800000 },
+ { .name = "l1pram", .dev_addr = 0xe00000 },
+ { .name = "l1dram", .dev_addr = 0xf00000 },
+};
+
+static const struct k3_dsp_dev_data c66_data = {
+ .mems = c66_mems,
+ .num_mems = ARRAY_SIZE(c66_mems),
+ .boot_align_addr = SZ_1K,
+ .uses_lreset = true,
+};
+
+static const struct of_device_id k3_dsp_of_match[] = {
+ { .compatible = "ti,j721e-c66-dsp", .data = &c66_data, },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, k3_dsp_of_match);
+
+static struct platform_driver k3_dsp_rproc_driver = {
+ .probe = k3_dsp_rproc_probe,
+ .remove = k3_dsp_rproc_remove,
+ .driver = {
+ .name = "k3-dsp-rproc",
+ .of_match_table = k3_dsp_of_match,
+ },
+};
+
+module_platform_driver(k3_dsp_rproc_driver);
+
+MODULE_AUTHOR("Suman Anna <s-anna@ti.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("TI K3 DSP Remoteproc driver");
--
2.26.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 0/4] TI K3 DSP remoteproc driver for C66x DSPs
From: Suman Anna @ 2020-05-21 0:10 UTC (permalink / raw)
To: Bjorn Andersson, Rob Herring, Mathieu Poirier
Cc: devicetree, Lokesh Vutla, linux-remoteproc, linux-kernel,
Suman Anna, linux-arm-kernel
Hi All,
The following is v2 of the K3 DSP remoteproc driver supporting the C66x DSPs
on the TI K3 J721E SoCs. The patches are based on the latest commit on the
rproc-next branch, 7dcef3988eed ("remoteproc: Fix an error code in
devm_rproc_alloc()").
v2 includes a new remoteproc core patch (patch 1) that adds an OF helper
for parsing the firmware-name property. This is refactored out to avoid
replicating the code in various remoteproc drivers. Please see the
individual patches for detailed changes.
The main dependent patches from the previous series are now staged in
rproc-next branch. The only dependency for this series is the common
ti-sci-proc helper between R5 and DSP drivers [1]. Please see the initial
cover-letter [2] for v1 details.
regards
Suman
[1] https://patchwork.kernel.org/patch/11456379/
[2] https://patchwork.kernel.org/cover/11458573/
Suman Anna (4):
remoteproc: Introduce rproc_of_parse_firmware() helper
dt-bindings: remoteproc: Add bindings for C66x DSPs on TI K3 SoCs
remoteproc/k3-dsp: Add a remoteproc driver of K3 C66x DSPs
remoteproc/k3-dsp: Add support for L2RAM loading on C66x DSPs
.../bindings/remoteproc/ti,k3-dsp-rproc.yaml | 190 +++++
drivers/remoteproc/Kconfig | 13 +
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/remoteproc_core.c | 23 +
drivers/remoteproc/remoteproc_internal.h | 2 +
drivers/remoteproc/ti_k3_dsp_remoteproc.c | 773 ++++++++++++++++++
6 files changed, 1002 insertions(+)
create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
create mode 100644 drivers/remoteproc/ti_k3_dsp_remoteproc.c
--
2.26.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox