* [PATCH 4/4] ARM: dts: fix PMC compatible
From: Alexandre Belloni @ 2018-06-07 8:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180607084107.4461-1-alexandre.belloni@bootlin.com>
Make each SoC dtsi use its soc specific PMc compatible string. This solves
a potential issue on at91sam9261 and at91sam9263 when using suspend to RAM
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
arch/arm/boot/dts/at91sam9261.dtsi | 2 +-
arch/arm/boot/dts/at91sam9263.dtsi | 2 +-
arch/arm/boot/dts/at91sam9rl.dtsi | 2 +-
arch/arm/boot/dts/sama5d4.dtsi | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/at91sam9261.dtsi b/arch/arm/boot/dts/at91sam9261.dtsi
index 53c63d0a418a..33f09d5ea020 100644
--- a/arch/arm/boot/dts/at91sam9261.dtsi
+++ b/arch/arm/boot/dts/at91sam9261.dtsi
@@ -590,7 +590,7 @@
};
pmc: pmc at fffffc00 {
- compatible = "atmel,at91rm9200-pmc", "syscon";
+ compatible = "atmel,at91sam9261-pmc", "syscon";
reg = <0xfffffc00 0x100>;
interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
interrupt-controller;
diff --git a/arch/arm/boot/dts/at91sam9263.dtsi b/arch/arm/boot/dts/at91sam9263.dtsi
index 87fb0660ab5d..af68a86c9973 100644
--- a/arch/arm/boot/dts/at91sam9263.dtsi
+++ b/arch/arm/boot/dts/at91sam9263.dtsi
@@ -93,7 +93,7 @@
};
pmc: pmc at fffffc00 {
- compatible = "atmel,at91rm9200-pmc", "syscon";
+ compatible = "atmel,at91sam9263-pmc", "syscon";
reg = <0xfffffc00 0x100>;
interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
interrupt-controller;
diff --git a/arch/arm/boot/dts/at91sam9rl.dtsi b/arch/arm/boot/dts/at91sam9rl.dtsi
index bd001cca25a4..8fb22030f00b 100644
--- a/arch/arm/boot/dts/at91sam9rl.dtsi
+++ b/arch/arm/boot/dts/at91sam9rl.dtsi
@@ -832,7 +832,7 @@
};
pmc: pmc at fffffc00 {
- compatible = "atmel,at91sam9g45-pmc", "syscon";
+ compatible = "atmel,at91sam9rl-pmc", "syscon";
reg = <0xfffffc00 0x100>;
interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
interrupt-controller;
diff --git a/arch/arm/boot/dts/sama5d4.dtsi b/arch/arm/boot/dts/sama5d4.dtsi
index 0cf9beddd556..92a35a1942b6 100644
--- a/arch/arm/boot/dts/sama5d4.dtsi
+++ b/arch/arm/boot/dts/sama5d4.dtsi
@@ -392,7 +392,7 @@
};
pmc: pmc at f0018000 {
- compatible = "atmel,sama5d3-pmc", "syscon";
+ compatible = "atmel,sama5d4-pmc", "syscon";
reg = <0xf0018000 0x120>;
interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
interrupt-controller;
--
2.17.1
^ permalink raw reply related
* [PATCH 3/4] ARM: at91: fix USB clock detection handling
From: Alexandre Belloni @ 2018-06-07 8:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180607084107.4461-1-alexandre.belloni@bootlin.com>
Add more compatibles to be able to correct the USB clock detection.
at91sam9261 and at91sam9263 have the same PMC_SCSR layout as at91sam9260.
at91sam9rl doesn't have any USB clock.
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
arch/arm/mach-at91/pm.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index 849014c01cf4..563f34d01ce4 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -559,15 +559,20 @@ static const struct pmc_info pmc_infos[] __initconst = {
{ .uhp_udp_mask = AT91RM9200_PMC_UHP | AT91RM9200_PMC_UDP },
{ .uhp_udp_mask = AT91SAM926x_PMC_UHP | AT91SAM926x_PMC_UDP },
{ .uhp_udp_mask = AT91SAM926x_PMC_UHP },
+ { .uhp_udp_mask = 0 },
};
static const struct of_device_id atmel_pmc_ids[] __initconst = {
{ .compatible = "atmel,at91rm9200-pmc", .data = &pmc_infos[0] },
{ .compatible = "atmel,at91sam9260-pmc", .data = &pmc_infos[1] },
+ { .compatible = "atmel,at91sam9261-pmc", .data = &pmc_infos[1] },
+ { .compatible = "atmel,at91sam9263-pmc", .data = &pmc_infos[1] },
{ .compatible = "atmel,at91sam9g45-pmc", .data = &pmc_infos[2] },
{ .compatible = "atmel,at91sam9n12-pmc", .data = &pmc_infos[1] },
+ { .compatible = "atmel,at91sam9rl-pmc", .data = &pmc_infos[3] },
{ .compatible = "atmel,at91sam9x5-pmc", .data = &pmc_infos[1] },
{ .compatible = "atmel,sama5d3-pmc", .data = &pmc_infos[1] },
+ { .compatible = "atmel,sama5d4-pmc", .data = &pmc_infos[1] },
{ .compatible = "atmel,sama5d2-pmc", .data = &pmc_infos[1] },
{ /* sentinel */ },
};
--
2.17.1
^ permalink raw reply related
* [PATCH 2/4] dt-bindings: clk: at91: Document all the PMC compatibles
From: Alexandre Belloni @ 2018-06-07 8:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180607084107.4461-1-alexandre.belloni@bootlin.com>
Add missing PMC compatibles to the list of available compatibles.
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
Documentation/devicetree/bindings/clock/at91-clock.txt | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/at91-clock.txt b/Documentation/devicetree/bindings/clock/at91-clock.txt
index 51c259a92d02..a1f591969538 100644
--- a/Documentation/devicetree/bindings/clock/at91-clock.txt
+++ b/Documentation/devicetree/bindings/clock/at91-clock.txt
@@ -17,14 +17,13 @@ Required properties:
"atmel,at91sam9x5-clk-slow-rc-osc":
at91 internal slow RC oscillator
- "atmel,at91rm9200-pmc" or
- "atmel,at91sam9g45-pmc" or
- "atmel,at91sam9n12-pmc" or
- "atmel,at91sam9x5-pmc" or
- "atmel,sama5d3-pmc":
+ "atmel,<chip>-pmc":
at91 PMC (Power Management Controller)
All at91 specific clocks (clocks defined below) must be child
node of the PMC node.
+ <chip> can be: at91rm9200, at91sam9260, at91sam9261,
+ at91sam9263, at91sam9g45, at91sam9n12, at91sam9rl, at91sam9x5,
+ sama5d2, sama5d3 or sama5d4.
"atmel,at91sam9x5-clk-slow" (under sckc node)
or
--
2.17.1
^ permalink raw reply related
* [PATCH 1/4] dt-bindings: arm: remove PMC bindings
From: Alexandre Belloni @ 2018-06-07 8:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180607084107.4461-1-alexandre.belloni@bootlin.com>
The PMC bindings are fully described in
Documentation/devicetree/bindings/clock/at91-clock.txt. Remove the
duplicate and incomplete documentation.
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
.../devicetree/bindings/arm/atmel-pmc.txt | 14 --------------
1 file changed, 14 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/arm/atmel-pmc.txt
diff --git a/Documentation/devicetree/bindings/arm/atmel-pmc.txt b/Documentation/devicetree/bindings/arm/atmel-pmc.txt
deleted file mode 100644
index 795cc78543fe..000000000000
--- a/Documentation/devicetree/bindings/arm/atmel-pmc.txt
+++ /dev/null
@@ -1,14 +0,0 @@
-* Power Management Controller (PMC)
-
-Required properties:
-- compatible: Should be "atmel,<chip>-pmc".
- <chip> can be: at91rm9200, at91sam9260, at91sam9g45, at91sam9n12,
- at91sam9x5, sama5d3
-
-- reg: Should contain PMC registers location and length
-
-Examples:
- pmc: pmc at fffffc00 {
- compatible = "atmel,at91rm9200-pmc";
- reg = <0xfffffc00 0x100>;
- };
--
2.17.1
^ permalink raw reply related
* [PATCH 0/4] Fix suspend resume on at91sam9261 and at91sam9263
From: Alexandre Belloni @ 2018-06-07 8:41 UTC (permalink / raw)
To: linux-arm-kernel
USB clock detection may not work properly on at91sam9261 and at91sam9263
because they currently use the same bit mask as at91rm9200 instead of
the one for at91sam9260.
Take the opportunity to also change the PMC compatible strings for all
the other SoCs in preparation for the new clock bindings.
Alexandre Belloni (4):
dt-bindings: arm: remove PMC bindings
dt-bindings: clk: at91: Document all the PMC compatibles
ARM: at91: fix USB clock detection handling
ARM: dts: fix PMC compatible
.../devicetree/bindings/arm/atmel-pmc.txt | 14 --------------
.../devicetree/bindings/clock/at91-clock.txt | 9 ++++-----
arch/arm/boot/dts/at91sam9261.dtsi | 2 +-
arch/arm/boot/dts/at91sam9263.dtsi | 2 +-
arch/arm/boot/dts/at91sam9rl.dtsi | 2 +-
arch/arm/boot/dts/sama5d4.dtsi | 2 +-
arch/arm/mach-at91/pm.c | 5 +++++
7 files changed, 13 insertions(+), 23 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/arm/atmel-pmc.txt
--
2.17.1
^ permalink raw reply
* [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
From: Viresh Kumar @ 2018-06-07 8:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180607074628.kd3iyxevwj3ypzbr@intel.com>
On 07-06-18, 15:46, Du, Changbin wrote:
> I think if the destination is not a null terminated string (If I understand your
> description below), memcpy can be used to get rid of such warning. The warning
> makes sense in general as explained in mannual. Thanks!
The destination should be a null terminated string eventually, but we first need
to make sure src is a null terminated string.
--
viresh
^ permalink raw reply
* [PATCH v4 05/14] coresight: get/put module in coresight_build/release_path
From: Greg Kroah-Hartman @ 2018-06-07 8:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180606155501.704583e1412996a1a2c6fa61@arm.com>
On Wed, Jun 06, 2018 at 03:55:01PM -0500, Kim Phillips wrote:
> On Wed, 6 Jun 2018 10:46:36 +0100
> Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>
> > On 06/06/2018 09:24 AM, Greg Kroah-Hartman wrote:
> > > On Tue, Jun 05, 2018 at 04:07:01PM -0500, Kim Phillips wrote:
> > >> Increment the refcnt for driver modules in current use by calling
> > >> module_get in coresight_build_path and module_put in release_path.
> > >>
> > >> This prevents driver modules from being unloaded when they are in use,
> > >> either in sysfs or perf mode.
> > >
> > > Why does it matter? Shouldn't you be allowed to remove any module at
> > > any point in time, much like a networking driver?
> > >
> > >
> > >>
> > >> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> > >> Cc: Leo Yan <leo.yan@linaro.org>
> > >> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > >> Cc: Randy Dunlap <rdunlap@infradead.org>
> > >> Cc: Suzuki K Poulose <Suzuki.Poulose@arm.com>
> > >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > >> Cc: Russell King <linux@armlinux.org.uk>
> > >> Signed-off-by: Kim Phillips <kim.phillips@arm.com>
> > >> ---
> > >> drivers/hwtracing/coresight/coresight.c | 9 +++++++++
> > >> 1 file changed, 9 insertions(+)
> > >>
> > >> diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
> > >> index 338f1719641c..1c941351f1d1 100644
> > >> --- a/drivers/hwtracing/coresight/coresight.c
> > >> +++ b/drivers/hwtracing/coresight/coresight.c
> > >> @@ -465,6 +465,12 @@ static int _coresight_build_path(struct coresight_device *csdev,
> > >>
> > >> node->csdev = csdev;
> > >> list_add(&node->link, path);
> > >> +
> > >> + if (!try_module_get(csdev->dev.parent->driver->owner)) {
> > >
> > > What is to keep parent->driver from going away right here? What keeps
> > > parent around? This feels very fragile to me, I don't see any locking
> > > anywhere around this code path to try to keep things in place.
> >
> > You're right. We do have coresight_mutex, which is held across the build
> > path and the csdev is removed when a device is unregistered. However, I
> > see that we don't hold the mutex while removing the connections from
> > coresight_unregister(). Holding the mutex should protect us from the
> > csdev being removed, while we build the path.
>
> OK, I'll add this for the next version:
>
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index f96258de1e9b..da702507a55c 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -1040,8 +1040,12 @@ EXPORT_SYMBOL_GPL(coresight_register);
>
> void coresight_unregister(struct coresight_device *csdev)
> {
> + mutex_lock(&coresight_mutex);
> +
Locks are to protect data, not code, be careful here please.
That's the big issue with the module reference counting, it "protects"
code, not data. If at all possible, never grab a module reference
count, as you should always be able to unload a module, unless you have
a file handle open, and if you have that, the kernel core will properly
protect you.
thanks,
greg k-h
^ permalink raw reply
* [PATCH][next] pinctrl: pinctrl-single: add allocation failure checking of saved_vals
From: Colin Ian King @ 2018-06-07 8:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180607073543.GP13775@localhost>
On 07/06/18 08:35, Johan Hovold wrote:
> On Wed, Jun 06, 2018 at 07:02:03PM +0300, Andy Shevchenko wrote:
>> On Wed, Jun 6, 2018 at 4:43 PM, Colin King <colin.king@canonical.com> wrote:
>>> From: Colin Ian King <colin.king@canonical.com>
>>>
>>> Currently saved_vals is being allocated and there is no check for
>>> failed allocation (which is more likely than normal when using
>>> GFP_ATOMIC). Fix this by checking for a failed allocation and
>>> propagating this error return down the the caller chain.
>>>
>>> Detected by CoverityScan, CID#1469841 ("Dereference null return value")
>>>
>>> Fixes: 88a1dbdec682 ("pinctrl: pinctrl-single: Add functions to save and restore pinctrl context")
>>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>>> ---
>>> drivers/pinctrl/pinctrl-single.c | 14 +++++++++++---
>>> 1 file changed, 11 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
>>> index 9c3c00515aa0..0905ee002041 100644
>>> --- a/drivers/pinctrl/pinctrl-single.c
>>> +++ b/drivers/pinctrl/pinctrl-single.c
>>> @@ -1588,8 +1588,11 @@ static int pcs_save_context(struct pcs_device *pcs)
>>>
>>> mux_bytes = pcs->width / BITS_PER_BYTE;
>>>
>>> - if (!pcs->saved_vals)
>>> + if (!pcs->saved_vals) {
>>> pcs->saved_vals = devm_kzalloc(pcs->dev, pcs->size, GFP_ATOMIC);
>>
>>> + if (!pcs->saved_vals)
>>> + return -ENOMEM;
>>
>> Wouldn't make sense to move it out of the first condition?
>>
>> Something like
>>
>> if (!foo)
>> foo = ...malloc(...);
>> if (!foo)
>> return ...
>
> No, checking for NULL immediately after the allocation is more obvious
> and easier to parse.
+1 on that
>
> Johan
>
^ permalink raw reply
* [v2, 09/10] dpaa_eth: add support for hardware timestamping
From: Madalin-cristian Bucur @ 2018-06-07 8:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180607032256.39802-10-yangbo.lu@nxp.com>
> -----Original Message-----
> From: Yangbo Lu [mailto:yangbo.lu at nxp.com]
> Sent: Thursday, June 7, 2018 6:23 AM
> Subject: [v2, 09/10] dpaa_eth: add support for hardware timestamping
>
> This patch is to add hardware timestamping support
> for dpaa_eth. On Rx, timestamping is enabled for
> all frames. On Tx, we only instruct the hardware
> to timestamp the frames marked accordingly by the
> stack.
>
> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> ---
> Changes for v2:
> - Removed ifdef for timestamp code.
> - Minor fixes for code style.
> ---
> drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 101
> ++++++++++++++++++++++-
> drivers/net/ethernet/freescale/dpaa/dpaa_eth.h | 3 +
> 2 files changed, 99 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> index fd43f98..bd589ac 100644
> --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> @@ -1168,7 +1168,7 @@ static int dpaa_eth_init_tx_port(struct fman_port
> *port, struct dpaa_fq *errq,
> buf_prefix_content.priv_data_size = buf_layout->priv_data_size;
> buf_prefix_content.pass_prs_result = true;
> buf_prefix_content.pass_hash_result = true;
> - buf_prefix_content.pass_time_stamp = false;
> + buf_prefix_content.pass_time_stamp = true;
> buf_prefix_content.data_align = DPAA_FD_DATA_ALIGNMENT;
>
> params.specific_params.non_rx_params.err_fqid = errq->fqid;
> @@ -1210,7 +1210,7 @@ static int dpaa_eth_init_rx_port(struct fman_port
> *port, struct dpaa_bp **bps,
> buf_prefix_content.priv_data_size = buf_layout->priv_data_size;
> buf_prefix_content.pass_prs_result = true;
> buf_prefix_content.pass_hash_result = true;
> - buf_prefix_content.pass_time_stamp = false;
> + buf_prefix_content.pass_time_stamp = true;
> buf_prefix_content.data_align = DPAA_FD_DATA_ALIGNMENT;
>
> rx_p = ¶ms.specific_params.rx_params;
> @@ -1592,6 +1592,16 @@ static int dpaa_eth_refill_bpools(struct dpaa_priv
> *priv)
> return 0;
> }
>
> +static int dpaa_get_tstamp_ns(struct net_device *net_dev, u64 *ns,
> + struct fman_port *port, const void *data)
> +{
> + if (!fman_port_get_tstamp_field(port, data, ns)) {
> + be64_to_cpus(ns);
Please move this endianness conversion in the fman API.
> + return 0;
> + }
> + return -EINVAL;
> +}
> +
> /* Cleanup function for outgoing frame descriptors that were built on Tx
> path,
> * either contiguous frames or scatter/gather ones.
> * Skb freeing is not handled here.
> @@ -1607,14 +1617,29 @@ static int dpaa_eth_refill_bpools(struct dpaa_priv
> *priv)
> {
> const enum dma_data_direction dma_dir = DMA_TO_DEVICE;
> struct device *dev = priv->net_dev->dev.parent;
> + struct skb_shared_hwtstamps shhwtstamps;
> dma_addr_t addr = qm_fd_addr(fd);
> const struct qm_sg_entry *sgt;
> struct sk_buff **skbh, *skb;
> int nr_frags, i;
> + u64 ns;
>
> skbh = (struct sk_buff **)phys_to_virt(addr);
> skb = *skbh;
>
> + if (priv->tx_tstamp && skb_shinfo(skb)->tx_flags &
> SKBTX_HW_TSTAMP) {
> + memset(&shhwtstamps, 0, sizeof(shhwtstamps));
> +
> + if (!dpaa_get_tstamp_ns(priv->net_dev, &ns,
> + priv->mac_dev->port[TX],
> + (void *)skbh)) {
> + shhwtstamps.hwtstamp = ns_to_ktime(ns);
> + skb_tstamp_tx(skb, &shhwtstamps);
> + } else {
> + dev_warn(dev, "dpaa_get_tstamp_ns failed!\n");
> + }
> + }
> +
> if (unlikely(qm_fd_get_format(fd) == qm_fd_sg)) {
> nr_frags = skb_shinfo(skb)->nr_frags;
> dma_unmap_single(dev, addr, qm_fd_get_offset(fd) +
> @@ -2086,6 +2111,11 @@ static int dpaa_start_xmit(struct sk_buff *skb,
> struct net_device *net_dev)
> if (unlikely(err < 0))
> goto skb_to_fd_failed;
>
> + if (priv->tx_tstamp && skb_shinfo(skb)->tx_flags &
> SKBTX_HW_TSTAMP) {
> + fd.cmd |= FM_FD_CMD_UPD;
The fd.cmd field is big endian, please use this:
+ fd.cmd |= cpu_to_be32(FM_FD_CMD_UPD);
> + skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
> + }
> +
> if (likely(dpaa_xmit(priv, percpu_stats, queue_mapping, &fd) == 0))
> return NETDEV_TX_OK;
>
> @@ -2227,6 +2257,7 @@ static enum qman_cb_dqrr_result
> rx_default_dqrr(struct qman_portal *portal,
> struct qman_fq *fq,
> const struct qm_dqrr_entry
> *dq)
> {
> + struct skb_shared_hwtstamps *shhwtstamps;
> struct rtnl_link_stats64 *percpu_stats;
> struct dpaa_percpu_priv *percpu_priv;
> const struct qm_fd *fd = &dq->fd;
> @@ -2240,6 +2271,7 @@ static enum qman_cb_dqrr_result
> rx_default_dqrr(struct qman_portal *portal,
> struct sk_buff *skb;
> int *count_ptr;
> void *vaddr;
> + u64 ns;
>
> fd_status = be32_to_cpu(fd->status);
> fd_format = qm_fd_get_format(fd);
> @@ -2304,6 +2336,18 @@ static enum qman_cb_dqrr_result
> rx_default_dqrr(struct qman_portal *portal,
> if (!skb)
> return qman_cb_dqrr_consume;
>
> + if (priv->rx_tstamp) {
> + shhwtstamps = skb_hwtstamps(skb);
> + memset(shhwtstamps, 0, sizeof(*shhwtstamps));
> +
> + if (!dpaa_get_tstamp_ns(priv->net_dev, &ns,
> + priv->mac_dev->port[RX],
> + vaddr))
> + shhwtstamps->hwtstamp = ns_to_ktime(ns);
> + else
> + dev_warn(net_dev->dev.parent,
> "dpaa_get_tstamp_ns failed!\n");
> + }
> +
> skb->protocol = eth_type_trans(skb, net_dev);
>
> if (net_dev->features & NETIF_F_RXHASH && priv->keygen_in_use
> &&
> @@ -2523,11 +2567,58 @@ static int dpaa_eth_stop(struct net_device
> *net_dev)
> return err;
> }
>
> +static int dpaa_ts_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
> +{
> + struct dpaa_priv *priv = netdev_priv(dev);
> + struct hwtstamp_config config;
> +
> + if (copy_from_user(&config, rq->ifr_data, sizeof(config)))
> + return -EFAULT;
> +
> + switch (config.tx_type) {
> + case HWTSTAMP_TX_OFF:
> + /* Couldn't disable rx/tx timestamping separately.
> + * Do nothing here.
> + */
> + priv->tx_tstamp = false;
> + break;
> + case HWTSTAMP_TX_ON:
> + priv->mac_dev->set_tstamp(priv->mac_dev->fman_mac,
> true);
> + priv->tx_tstamp = true;
> + break;
> + default:
> + return -ERANGE;
> + }
> +
> + if (config.rx_filter == HWTSTAMP_FILTER_NONE) {
> + /* Couldn't disable rx/tx timestamping separately.
> + * Do nothing here.
> + */
> + priv->rx_tstamp = false;
> + } else {
> + priv->mac_dev->set_tstamp(priv->mac_dev->fman_mac,
> true);
> + priv->rx_tstamp = true;
> + /* TS is set for all frame types, not only those requested */
> + config.rx_filter = HWTSTAMP_FILTER_ALL;
> + }
> +
> + return copy_to_user(rq->ifr_data, &config, sizeof(config)) ?
> + -EFAULT : 0;
> +}
> +
> static int dpaa_ioctl(struct net_device *net_dev, struct ifreq *rq, int cmd)
> {
> - if (!net_dev->phydev)
> - return -EINVAL;
> - return phy_mii_ioctl(net_dev->phydev, rq, cmd);
> + int ret = -EINVAL;
> +
> + if (cmd == SIOCGMIIREG) {
> + if (net_dev->phydev)
> + return phy_mii_ioctl(net_dev->phydev, rq, cmd);
> + }
> +
> + if (cmd == SIOCSHWTSTAMP)
> + return dpaa_ts_ioctl(net_dev, rq, cmd);
> +
> + return ret;
> }
>
> static const struct net_device_ops dpaa_ops = {
> diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
> b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
> index bd94220..af320f8 100644
> --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
> +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
> @@ -182,6 +182,9 @@ struct dpaa_priv {
>
> struct dpaa_buffer_layout buf_layout[2];
> u16 rx_headroom;
> +
> + bool tx_tstamp; /* Tx timestamping enabled */
> + bool rx_tstamp; /* Rx timestamping enabled */
> };
>
> /* from dpaa_ethtool.c */
> --
> 1.7.1
^ permalink raw reply
* [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
From: Johan Hovold @ 2018-06-07 8:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180607041718.qpqucjzlvcm5h3gn@vireshk-i7>
On Thu, Jun 07, 2018 at 09:47:18AM +0530, Viresh Kumar wrote:
> On 06-06-18, 14:26, Steven Rostedt wrote:
> > On Wed, 6 Jun 2018 16:26:00 +0200
> > Johan Hovold <johan@kernel.org> wrote:
> >
> > > Looks like the greybus code above is working as intended by checking for
> > > unterminated string after the strncpy, even if this does now triggers
> > > the truncation warning.
>
> So why exactly are we generating a warning here ? Is it because it is possible
> that the first n bytes of src may not have the null terminating byte and the
> dest may not be null terminated eventually ?
Yes, new warning in GCC 8:
https://gcc.gnu.org/onlinedocs/gcc-8.1.0/gcc/Warning-Options.html#index-Wstringop-truncation
> Maybe I should just use memcpy here then ?
No, as you note below, you use strncpy to clear the rest of the buffer.
> But AFAIR, I used strncpy() specifically because it also sets all the remaining
> bytes after the null terminating byte with the null terminating byte. And so it
> is pretty easy for me to check if the final string is null terminated by
> checking [max - 1] byte against '\0', which the code is doing right now.
>
> I am not sure what would the best way to get around this incorrect-warning.
It seems gcc just isn't smart enough in this case (where you check for
overflow and never use a non-terminated string), but it is supposed to
detect when the string is unconditionally terminated. So perhaps just
adding a redundant buf[size-1] = '\0' before returning in the error path
or after the error path would shut it up. But that's a bit of a long
shot, I admit.
Probably best to leave things as they are, and let the gcc folks find a
way to handle such false positives.
Thanks,
Johan
^ permalink raw reply
* [PATCH v13 0/3] Fix issues with huge mapping in ioremap for ARM64
From: Chintan Pandya @ 2018-06-07 8:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180606154520.GM6631@arm.com>
On 6/6/2018 9:15 PM, Will Deacon wrote:
> Hi Chintan,
Hi Will,
>
> Thanks for sticking with this. I've reviewed the series now and I'm keen
> for it to land in mainline. Just a couple of things below.
>
Thanks for all the reviews so far.
> On Wed, Jun 06, 2018 at 12:31:18PM +0530, Chintan Pandya wrote:
>> This series of patches re-bring huge vmap back for arm64.
...
>> These patches can also go into '-stable' branch (if accepted)
>> for 4.6 onwards.
>
> Not sure we need to target -stable, since we solved the crash by disabling
> the use of huge io mappings.
If disabling of huge io mappings have gone into stable trees, then I
won't push for these changes there.
>
>> arch/arm64/include/asm/tlbflush.h | 7 ++++++
>> arch/arm64/mm/mmu.c | 48 +++++++++++++++++++++++++++++++++++----
>> arch/x86/mm/pgtable.c | 8 ++++---
>> include/asm-generic/pgtable.h | 8 +++----
>> lib/ioremap.c | 4 ++--
>
> If you get an ack from the x86 folks, then I could take all of this via
> arm64. Alternatively, now that I've reviewed the series this could happily
> go via another tree (e.g. akpm).
Sure. I would wait for few days before either of them take notice of
this. If required, I will communicated with them.
>
> Thanks,
>
> Will
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
Chintan
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation
Collaborative Project
^ permalink raw reply
* [PATCH v1 2/4] clk: rockchip: add dt-binding header for px30
From: Heiko Stübner @ 2018-06-07 7:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1528340786-462-3-git-send-email-zhangqing@rock-chips.com>
Am Donnerstag, 7. Juni 2018, 05:06:24 CEST schrieb Elaine Zhang:
> Add the dt-bindings header for the px30, that gets shared between
> the clock controller and the clock references in the dts.
> Add softreset ID for px30.
>
> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
> ---
> include/dt-bindings/clock/px30-cru.h | 402
> +++++++++++++++++++++++++++++++++++ 1 file changed, 402 insertions(+)
> create mode 100644 include/dt-bindings/clock/px30-cru.h
>
> diff --git a/include/dt-bindings/clock/px30-cru.h
> b/include/dt-bindings/clock/px30-cru.h new file mode 100644
> index 000000000000..db9fc2a0bb21
> --- /dev/null
> +++ b/include/dt-bindings/clock/px30-cru.h
> @@ -0,0 +1,402 @@
> +/*
> + * Copyright (c) 2017 Rockchip Electronics Co. Ltd.
> + * Author: Elaine <zhangqing@rock-chips.com>
^^ full name maybe?
Otherwise looks good.
Heiko
^ permalink raw reply
* [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
From: Du, Changbin @ 2018-06-07 7:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180607041718.qpqucjzlvcm5h3gn@vireshk-i7>
Hi,
On Thu, Jun 07, 2018 at 09:47:18AM +0530, Viresh Kumar wrote:
> +Greg/Alex,
>
> @Fegguang/build-bot: I do see mention of Greg and /me in your initial email's
> body saying TO: Viresh, CC: Greg, but I don't see any of us getting cc'd in your
> email. Bug ?
>
> On 06-06-18, 14:26, Steven Rostedt wrote:
> > On Wed, 6 Jun 2018 16:26:00 +0200
> > Johan Hovold <johan@kernel.org> wrote:
> >
> > > Looks like the greybus code above is working as intended by checking for
> > > unterminated string after the strncpy, even if this does now triggers
> > > the truncation warning.
>
> So why exactly are we generating a warning here ? Is it because it is possible
> that the first n bytes of src may not have the null terminating byte and the
> dest may not be null terminated eventually ?
>
> Maybe I should just use memcpy here then ?
>
I think if the destination is not a null terminated string (If I understand your
description below), memcpy can be used to get rid of such warning. The warning
makes sense in general as explained in mannual. Thanks!
> But AFAIR, I used strncpy() specifically because it also sets all the remaining
> bytes after the null terminating byte with the null terminating byte. And so it
> is pretty easy for me to check if the final string is null terminated by
> checking [max - 1] byte against '\0', which the code is doing right now.
>
> I am not sure what would the best way to get around this incorrect-warning.
>
> And I am wondering on why buildbot reported the warning only for two instances
> in that file, while I have done the same thing at 4 places.
>
> > Ah, yes I now see that. Thanks for pointing it out. But perhaps it
> > should also add the "- 1" to the strncpy() so that gcc doesn't think
> > it's a mistake.
>
> The src string is passed on from a firmware entity and we need to make sure the
> protocol (greybus) is implemented properly by the other end. For example, in the
> current case if the firmware sends "HELLOWORLD", its an error as it should have
> sent "HELLWORLD\0". But with what you are saying we will forcefully make dest as
> "HELLWORLD\0", which wouldn't be the right thing to do as we will miss the bug
> present in firmware.
>
> --
> viresh
--
Thanks,
Changbin Du
^ permalink raw reply
* [PATCH][next] pinctrl: pinctrl-single: add allocation failure checking of saved_vals
From: Johan Hovold @ 2018-06-07 7:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAHp75Vd0Djw+nDN7u=-CLEfdDGA5QTsMueOb9EZLNnCDgso5fA@mail.gmail.com>
On Wed, Jun 06, 2018 at 07:02:03PM +0300, Andy Shevchenko wrote:
> On Wed, Jun 6, 2018 at 4:43 PM, Colin King <colin.king@canonical.com> wrote:
> > From: Colin Ian King <colin.king@canonical.com>
> >
> > Currently saved_vals is being allocated and there is no check for
> > failed allocation (which is more likely than normal when using
> > GFP_ATOMIC). Fix this by checking for a failed allocation and
> > propagating this error return down the the caller chain.
> >
> > Detected by CoverityScan, CID#1469841 ("Dereference null return value")
> >
> > Fixes: 88a1dbdec682 ("pinctrl: pinctrl-single: Add functions to save and restore pinctrl context")
> > Signed-off-by: Colin Ian King <colin.king@canonical.com>
> > ---
> > drivers/pinctrl/pinctrl-single.c | 14 +++++++++++---
> > 1 file changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
> > index 9c3c00515aa0..0905ee002041 100644
> > --- a/drivers/pinctrl/pinctrl-single.c
> > +++ b/drivers/pinctrl/pinctrl-single.c
> > @@ -1588,8 +1588,11 @@ static int pcs_save_context(struct pcs_device *pcs)
> >
> > mux_bytes = pcs->width / BITS_PER_BYTE;
> >
> > - if (!pcs->saved_vals)
> > + if (!pcs->saved_vals) {
> > pcs->saved_vals = devm_kzalloc(pcs->dev, pcs->size, GFP_ATOMIC);
>
> > + if (!pcs->saved_vals)
> > + return -ENOMEM;
>
> Wouldn't make sense to move it out of the first condition?
>
> Something like
>
> if (!foo)
> foo = ...malloc(...);
> if (!foo)
> return ...
No, checking for NULL immediately after the allocation is more obvious
and easier to parse.
Johan
^ permalink raw reply
* [PATCH v3 4/6] bus: ti-sysc: Add support for software reset
From: Tony Lindgren @ 2018-06-07 7:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180606060826.14671-5-faiz_abbas@ti.com>
* Faiz Abbas <faiz_abbas@ti.com> [180606 06:14]:
> +static int sysc_reset(struct sysc *ddata)
> +{
> + int offset = ddata->offsets[SYSC_SYSCONFIG];
> + int val = sysc_read(ddata, offset);
> +
> + val |= (0x1 << ddata->cap->regbits->srst_shift);
> + sysc_write(ddata, offset, val);
> +
> + /* Poll on reset status */
> + if (ddata->cfg.quirks & SYSC_QUIRK_RESET_STATUS) {
> + offset = ddata->offsets[SYSC_SYSSTATUS];
> +
> + return readl_poll_timeout(ddata->module_va + offset, val,
> + (val & ddata->cfg.syss_mask) == 0x0,
> + 100, MAX_MODULE_SOFTRESET_WAIT);
> + }
> +
> + return 0;
> +}
I wonder if we should also add SYSS_QUIRK_RESET_STATUS in
addition to SYSC_QUIRK_RESET status to make it easy to
read the right register?
Then we can add support for SYSC_QUIRK_RESET_STATUS later on
when tested and return error for now.
Regards,
Tony
^ permalink raw reply
* [PATCH v2 3/5] arm_pmu: Add support for 64bit event counters
From: Suzuki K Poulose @ 2018-06-07 7:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180606164838.zeuygsp4teq64zor@lakrids.cambridge.arm.com>
On 06/06/2018 05:48 PM, Mark Rutland wrote:
> On Tue, May 29, 2018 at 11:55:54AM +0100, Suzuki K Poulose wrote:
>> Each PMU has a set of 32bit event counters. But in some
>> special cases, the events could be counted using counters
>> which are effectively 64bit wide.
>>
>> e.g, Arm V8 PMUv3 has a 64 bit cycle counter which can count
>> only the CPU cycles. Also, the PMU can chain the event counters
>> to effectively count as a 64bit counter.
>>
>> Add support for tracking the events that uses 64bit counters.
>> This only affects the periods set for each counter in the core
>> driver.
>>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>> Changes since v1:
>> - Rename ARMPMU_EVT_LONG => ARMPMU_EVT_64BIT
>> ---
>> drivers/perf/arm_pmu.c | 14 ++++++++------
>> include/linux/perf/arm_pmu.h | 6 ++++++
>> 2 files changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
>> index 8962d26..ff858e6 100644
>> --- a/drivers/perf/arm_pmu.c
>> +++ b/drivers/perf/arm_pmu.c
>> @@ -28,9 +28,10 @@
>> static DEFINE_PER_CPU(struct arm_pmu *, cpu_armpmu);
>> static DEFINE_PER_CPU(int, cpu_irq);
>>
>> -static inline u64 arm_pmu_max_period(void)
>> +static inline u64 arm_pmu_event_max_period(struct perf_event *event)
>> {
>> - return (1ULL << 32) - 1;
>> + return (event->hw.flags & ARMPMU_EVT_64BIT) ?
>> + ~0ULL : (1ULL << 32) - 1;
>> }
>
> Could we please have:
>
> static inline u64 arm_pmu_event_max_period(struct perf_event *event)
> {
> if (event->hw.flags & ARMPMU_EVT_64BIT)
> return GENMASK_ULL(63, 0);
> else
> return GENMASK_ULL(31, 0);
> }
>
> ... since that's obviously balanced, with both values generated in the
> same way.
>
Sure, will do
Suzuki
^ permalink raw reply
* [PATCH][next] pinctrl: pinctrl-single: add allocation failure checking of saved_vals
From: Johan Hovold @ 2018-06-07 7:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180606134338.4645-1-colin.king@canonical.com>
On Wed, Jun 06, 2018 at 02:43:38PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Currently saved_vals is being allocated and there is no check for
> failed allocation (which is more likely than normal when using
> GFP_ATOMIC). Fix this by checking for a failed allocation and
> propagating this error return down the the caller chain.
>
> Detected by CoverityScan, CID#1469841 ("Dereference null return value")
> Fixes: 88a1dbdec682 ("pinctrl: pinctrl-single: Add functions to save and restore pinctrl context")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/pinctrl/pinctrl-single.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
> index 9c3c00515aa0..0905ee002041 100644
> --- a/drivers/pinctrl/pinctrl-single.c
> +++ b/drivers/pinctrl/pinctrl-single.c
> @@ -1588,8 +1588,11 @@ static int pcs_save_context(struct pcs_device *pcs)
>
> mux_bytes = pcs->width / BITS_PER_BYTE;
>
> - if (!pcs->saved_vals)
> + if (!pcs->saved_vals) {
> pcs->saved_vals = devm_kzalloc(pcs->dev, pcs->size, GFP_ATOMIC);
> + if (!pcs->saved_vals)
> + return -ENOMEM;
> + }
>
> switch (pcs->width) {
> case 64:
> @@ -1649,8 +1652,13 @@ static int pinctrl_single_suspend(struct platform_device *pdev,
> if (!pcs)
> return -EINVAL;
>
> - if (pcs->flags & PCS_CONTEXT_LOSS_OFF)
> - pcs_save_context(pcs);
> + if (pcs->flags & PCS_CONTEXT_LOSS_OFF) {
> + int ret;
> +
> + ret = pcs_save_context(pcs);
> + if (ret < 0)
> + return ret;
> + }
This appears to be the right fix (along the lines of what the author may
have intended by having the helper return an int), but as a follow-up
patch, why not move the allocation to probe() instead?
Also this doesn't look like something that requires atomic allocation in
the first place, GFP_KERNEL should do for the legacy suspend callback.
> return pinctrl_force_sleep(pcs->pctl);
> }
But for this fix, feel free to add:
Reviewed-by: Johan Hovold <johan@kernel.org>
Thanks,
Johan
^ permalink raw reply
* [PATCH 4/4] soc: imx: add SC firmware IPC and APIs
From: Sascha Hauer @ 2018-06-07 7:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <AM0PR04MB4211E378DCBB769AB2C06BCF80640@AM0PR04MB4211.eurprd04.prod.outlook.com>
On Thu, Jun 07, 2018 at 04:18:54AM +0000, A.s. Dong wrote:
> Hi Sascha,
>
> > > One problem of the way you suggested may be that:
> > > If we doing like below, we may lose flexibility to change the MU used
> > > for SCU firmware communication.
> > > scu at 5d1b0000 {
> > > compatible = "fsl,imx8qxp-scu";
> > > reg = <0x0 0x5d1b0000 0x0 0x10000>;
> > > };
> > >
> > > And current design is that the system supports multiple MU channels
> > > used by various users at the same time, e.g. SCU, Power Domain, Clock and
> > others.
> > > User can flexibly change it under their nodes: And each MU channel is
> > > protected by their private lock and not affect each others.
> > >
> > > e.g.
> > > scu {
> > > compatible = "nxp,imx8qxp-scu", "simple-bus";
> > > fsl,mu = <&lsio_mu0>;
> > >
> > > clk: clk {
> > > compatible = "fsl,imx8qxp-clk";
> > > #clock-cells = <1>;
> > > };
> > >
> > > iomuxc: iomuxc {
> > > compatible = "fsl,imx8qxp-iomuxc";
> > > fsl,mu = <&lsio_mu3>;
> > > };
> > >
> > > imx8qx-pm {
> > > #address-cells = <1>;
> > > #size-cells = <0>;
> > > fsl,mu = <&lsio_mu4>;
> > > .............
> > > }
> > >
> > > The default code only uses MU0 which is used by SCU.
> > >
> > > The change may affect this design. Any ideas?
> >
> > Sorry for the delay.
> >
> > You can add the child nodes to the mu nodes they should use:
> >
> > scu1 {
> > compatible = "nxp,imx8qxp-scu";
> > reg = <0x0 0x5d1b0000 0x0 0x10000>;
> >
> > clk: clk {
> > compatible = "fsl,imx8qxp-clk";
> > #clock-cells = <1>;
> > };
> >
> > ...
> > };
> >
> > scu2 {
> > compatible = "nxp,imx8qxp-scu";
> > reg = <0x0 someothermu 0x0 0x10000>;
> >
> > iomuxc: iomuxc {
> > compatible = "fsl,imx8qxp-iomuxc";
> > };
> >
> > ...
> > };
> >
> > So instead of adding all possible children to a single mu and phandle to other
> > mu's, just add the right children to each mu.
> >
>
> I got your point now. But sorry i'm still a bit hestitate to it....
>
> This way increases complexity and looks more like a per-channel binding.
> But we normally have only one (abstract) SCU firmware node in a system which may
> use different channels to implement different functions like clk, pd and etc.
> Multiple faked SCU nodes make people a bit confusing.
They are not faked, indeed that's the MU units that physically exist.
> Furthermore, it's still lose the flexibility for user to changing a MU to use.
>
> Looking at all exist users in kernel, seems no one to use like this.
> See:
> Documentation/devicetree/bindings/arm/arm,scpi.txt
> Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>
> All are similar like:
> xxx: protocol-node {
> compatible = "xxx-protocal";
> channel = ...
> ...
>
> clk_node: clk_node {
> ...
> };
>
> pd_node: pd_node {
> ...
> };
> };
> The protocol node work is selecting the correct channel, do necessary initialization
> and populate the all child function device nodes.
>
> IMHO I'm still a bit like to this common way used in kernel if no stronger objection.
> Do you think we can choose this way to go step forward?
I'm not convinced, but go ahead if you think this is the better way to
proceed.
I think my original point that led to this discussion is the muddled way
the MUs are handled in the code.
To start with in the system controller code you ioremap the physical
address of the MU and later on pass this address as a reference to
the MU library code. There's no way for the MU code to ever create a
private data. It would be much better if you would pass mu_init a
pointer to the device node it shall initialize, let mu_init allocate
a private data struct, ioremap the base and put it in the private data
struct, and return the private data struct.
Then there is this sc_ipc_get_handle() thing that your consumers shall
use to get a handle to the SCU. Instead of returning a struct sc_ipc *
there you return a ida which you first have to search for each time
a consumer wants to do something on the SCU. Please just return a
pointer there (which can be a cookie, i.e. the struct definition is
unknown to the consumer but privately to the SCU code).
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* [PATCH v4 2/3] arm: shmobile: Add the R9A06G032 SMP enabler driver
From: Michel Pollet @ 2018-06-07 6:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <79c0899e-7df1-1fe7-9681-ad3bd51feda7@gmail.com>
On 06 June 2018 22:53, Frank wrote:
> On 06/06/18 14:48, Frank Rowand wrote:
> > On 06/05/18 23:36, Michel Pollet wrote:
> >> Hi Frank,
> >>
> >> On 05 June 2018 18:34, Frank wrote:
> >>> On 06/05/18 04:28, Michel Pollet wrote:
> >>>> The Renesas R9A06G032 second CA7 is parked in a ROM pen at boot
> >>>> time, it requires a special enable method to get it started.
> >>>>
> >>>> Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
> >>>> ---
> >>>> arch/arm/mach-shmobile/Makefile | 1 +
> >>>> arch/arm/mach-shmobile/smp-r9a06g032.c | 79
> >>>> ++++++++++++++++++++++++++++++++++
> >>>> 2 files changed, 80 insertions(+)
> >>>> create mode 100644 arch/arm/mach-shmobile/smp-r9a06g032.c
> >>>>
> >>>> diff --git a/arch/arm/mach-shmobile/Makefile
> >>>> b/arch/arm/mach-shmobile/Makefile index 1939f52..d7fc98f 100644
> >>>> --- a/arch/arm/mach-shmobile/Makefile
> >>>> +++ b/arch/arm/mach-shmobile/Makefile
> >>>> @@ -34,6 +34,7 @@ smp-$(CONFIG_ARCH_SH73A0)+= smp-sh73a0.o
> >>> headsmp-scu.o platsmp-scu.o
> >>>> smp-$(CONFIG_ARCH_R8A7779)+= smp-r8a7779.o headsmp-scu.o
> >>> platsmp-scu.o
> >>>> smp-$(CONFIG_ARCH_R8A7790)+= smp-r8a7790.o
> >>>> smp-$(CONFIG_ARCH_R8A7791)+= smp-r8a7791.o
> >>>> +smp-$(CONFIG_ARCH_R9A06G032)+= smp-r9a06g032.o
> >>>> smp-$(CONFIG_ARCH_EMEV2)+= smp-emev2.o headsmp-scu.o
> >>> platsmp-scu.o
> >>>>
> >>>> # PM objects
> >>>> diff --git a/arch/arm/mach-shmobile/smp-r9a06g032.c
> >>>> b/arch/arm/mach-shmobile/smp-r9a06g032.c
> >>>> new file mode 100644
> >>>> index 0000000..cd40e6e
> >>>> --- /dev/null
> >>>> +++ b/arch/arm/mach-shmobile/smp-r9a06g032.c
> >>>> @@ -0,0 +1,79 @@
> >>>> +// SPDX-License-Identifier: GPL-2.0
> >>>> +/*
> >>>> + * R9A06G032 Second CA7 enabler.
> >>>> + *
> >>>> + * Copyright (C) 2018 Renesas Electronics Europe Limited
> >>>> + *
> >>>> + * Michel Pollet <michel.pollet@bp.renesas.com>,
> >>> <buserror@gmail.com>
> >>>> + * Derived from action,s500-smp
> >>>> + */
> >>>> +
> >>>> +#include <linux/io.h>
> >>>> +#include <linux/of.h>
> >>>> +#include <linux/of_address.h>
> >>>> +#include <linux/smp.h>
> >>>> +
> >>>> +/*
> >>>> + * The second CPU is parked in ROM at boot time. It requires
> >>>> +waking it after
> >>>> + * writing an address into the BOOTADDR register of sysctrl.
> >>>> + *
> >>>> + * So the default value of the "cpu-release-addr" corresponds to
> >>> BOOTADDR...
> >>>> + *
> >>>> + * *However* the BOOTADDR register is not available when the
> >>>> +kernel
> >>>> + * starts in NONSEC mode.
> >>>> + *
> >>>> + * So for NONSEC mode, the bootloader re-parks the second CPU into
> >>>> +a pen
> >>>> + * in SRAM, and changes the "cpu-release-addr" of linux's DT to a
> >>>> +SRAM address,
> >>>> + * which is not restricted.
> >>>
> >>> The binding document for cpu-release-addr does not have a definition
> >>> for 32 bit arm. The existing definition is only 64 bit arm. Please
> >>> add the definition for 32 bit arm to patch 1.
> >>
> >> Hmmm I do find a definition in
> >> Documentation/devicetree/bindings/arm/cpus.txt -- just under where I
> >> added my 'enable-method' -- And it is already used as 32 bits in at
> >> least arch/arm/boot/dts/stih407-family.dtsi.
> >
> > If the correct answer is for cpu-release-addr to be 64 bits in certain
> > cases (that discussion is ongoing further downthread) then one
> > approach to maintain compatibility _and_ to fix the devicetree source
> > files is to change the source code that currently gets
> > cpu-release-addr as a
> > 32 bit object to check the size of the property and get it as either a
> > 32 bit or 64 bit object, based on the actual size of the property in
> > the device tree and then change the value in the devicetree source
> > files to be two cells. BUT this does not consider the bootloader
> > complication. arch/arm/boot/dts/axm5516-cpus.dtsi has a note "//
> > Fixed by the boot loader", so the boot loader also has to be modified
> > to be able to handle the possibility that the property could be either
> > 32 bits or 64 bits. I don't know how to maintain compatibility with
> > the boot loader since we can't force it to change synchronously with
> > changes in the kernel.
> >
> > You can consider this comment to be a drive-by observation. I think
> > Rob and Geert and people like that are likely to be more helpful with
> > what to actually do, and you can treat my comment more as pointing out
> > the issue than as providing the perfect solution.
>
> Darn it, hit <send> too quickly.
>
> I meant to mention that there are several devicetree source files that have a
> single cell value for cpu-release-addr, and thus potentially face the same
> situation, depending on what the final decision is on the proper size for cpu-
> release-addr. As of v4.17, a git grep shows one cell values in:
>
> arch/arm/boot/dts/axm5516-cpus.dtsi
> arch/arm/boot/dts/stih407-family.dtsi
> arch/arm/boot/dts/stih418.dtsi
Yes, I had grepped before I used 32 bits on mine...
Now, what is the decision here? Our bootloader is already modified to set it to 32 bits, so I propose that
+ I change the driver to handle 32 and 64 bits properties
+ I add this to the cpu.txt, as a separate patch:
# On other systems, the property can be either
32 bits or 64 bits, it is the driver's responsibility
to deal with either sizes.
Michel
>
> -Frank
>
> > -Frnak
> >
> >
> >>
> >> What do you want me to add to this exactly? Do you want me to just
> >> change "required for systems that have an "enable-method" property
> >> value of "spin-table" to also specify renesas,r9a06g032 ?
> >>
> >> Thanks!
> >> Michel
> >>
> >>>
> >>> -Frank
> >>>
> >>>
> >>>> + */
> >>>> +
> >>>> +static void __iomem *cpu_bootaddr;
> >>>> +
> >>>> +static DEFINE_SPINLOCK(cpu_lock);
> >>>> +
> >>>> +static int r9a06g032_smp_boot_secondary(unsigned int cpu, struct
> >>>> +task_struct *idle) { if (!cpu_bootaddr) return -ENODEV;
> >>>> +
> >>>> +spin_lock(&cpu_lock);
> >>>> +
> >>>> +writel(__pa_symbol(secondary_startup), cpu_bootaddr);
> >>>> +arch_send_wakeup_ipi_mask(cpumask_of(cpu));
> >>>> +
> >>>> +spin_unlock(&cpu_lock);
> >>>> +
> >>>> +return 0;
> >>>> +}
> >>>> +
> >>>> +static void __init r9a06g032_smp_prepare_cpus(unsigned int
> >>>> +max_cpus) { struct device_node *dn; int ret;
> >>>> +u32 bootaddr;
> >>>> +
> >>>> +dn = of_get_cpu_node(1, NULL);
> >>>> +if (!dn) {
> >>>> +pr_err("CPU#1: missing device tree node\n"); return; }
> >>>> +/*
> >>>> + * Determine the address from which the CPU is polling.
> >>>> + * The bootloader *does* change this property */ ret =
> >>>> +of_property_read_u32(dn, "cpu-release-addr", &bootaddr);
> >>>> +of_node_put(dn); if (ret) {
> >>>> +pr_err("CPU#1: invalid cpu-release-addr property\n"); return; }
> >>>> +pr_info("CPU#1: cpu-release-addr %08x\n", bootaddr);
> >>>> +
> >>>> +cpu_bootaddr = ioremap(bootaddr, sizeof(bootaddr)); }
> >>>> +
> >>>> +static const struct smp_operations r9a06g032_smp_ops __initconst =
> >>>> +{ .smp_prepare_cpus = r9a06g032_smp_prepare_cpus,
> >>>> +.smp_boot_secondary = r9a06g032_smp_boot_secondary, };
> >>>> +CPU_METHOD_OF_DECLARE(r9a06g032_smp, "renesas,r9a06g032-
> smp",
> >>>> +&r9a06g032_smp_ops);
> >>>>
> >>
> >>
> >>
> >>
> >> Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne
> End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under
> Registered No. 04586709.
> >>
> >
> >
Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
^ permalink raw reply
* [PATCH] pwm: stm32: fix build warning with CONFIG_DMA_ENGINE disabled
From: Lee Jones @ 2018-06-07 5:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180606081827.GD11810@ulmo>
On Wed, 06 Jun 2018, Thierry Reding wrote:
> On Fri, May 25, 2018 at 11:08:30PM +0200, Arnd Bergmann wrote:
> > Without dmaengine support, we get a harmless warning about an
> > unused function:
> >
> > drivers/pwm/pwm-stm32.c:166:12: error: 'stm32_pwm_capture' defined but not used [-Werror=unused-function]
> >
> > Changing the #ifdef to an IS_ENABLED() check shuts up that warning
> > and is slightly nicer to read.
> >
> > Fixes: 53e38fe73f94 ("pwm: stm32: Add capture support")
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > drivers/pwm/pwm-stm32.c | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
>
> Applied, thanks.
>
> Lee, I applied this on top of your immutable MFD/PWM branch because it
> depends on the capture support that you applied. I wasn't sure what your
> PR timing was going to be, so I thought I'd do it this way since I'm
> pulling in some last minute fixes for v4.18.
I don't see a problem with that.
--
Lee Jones [???]
Linaro Services Technical Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH 4/4] soc: imx: add SC firmware IPC and APIs
From: A.s. Dong @ 2018-06-07 4:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180606141501.6prypkkxgsk3e7ee@pengutronix.de>
Hi Sascha,
> -----Original Message-----
> From: Sascha Hauer [mailto:s.hauer at pengutronix.de]
> Sent: Wednesday, June 6, 2018 10:15 PM
> To: A.s. Dong <aisheng.dong@nxp.com>
> Cc: dongas86 at gmail.com; dl-linux-imx <linux-imx@nxp.com>;
> kernel at pengutronix.de; Fabio Estevam <fabio.estevam@nxp.com>;
> shawnguo at kernel.org; linux-arm-kernel at lists.infradead.org
> Subject: Re: [PATCH 4/4] soc: imx: add SC firmware IPC and APIs
>
> On Thu, May 24, 2018 at 08:56:15AM +0000, A.s. Dong wrote:
> > Hi Sascha,
> >
> > > -----Original Message-----
> > > From: Sascha Hauer [mailto:s.hauer at pengutronix.de]
> > > Sent: Thursday, May 3, 2018 8:41 PM
> > > To: A.s. Dong <aisheng.dong@nxp.com>
> > > Cc: linux-arm-kernel at lists.infradead.org; dongas86 at gmail.com;
> > > dl-linux-imx <linux-imx@nxp.com>; kernel at pengutronix.de; Fabio
> > > Estevam <fabio.estevam@nxp.com>; shawnguo at kernel.org
> > > Subject: Re: [PATCH 4/4] soc: imx: add SC firmware IPC and APIs
> > >
> > > On Thu, May 03, 2018 at 12:29:40PM +0000, A.s. Dong wrote:
> > > > > -----Original Message-----
> > > > > From: Sascha Hauer [mailto:s.hauer at pengutronix.de]
> > > > > Sent: Thursday, May 3, 2018 7:06 PM
> > > > > To: A.s. Dong <aisheng.dong@nxp.com>
> > > > > Cc: linux-arm-kernel at lists.infradead.org; dongas86 at gmail.com;
> > > > > dl-linux-imx <linux-imx@nxp.com>; kernel at pengutronix.de; Fabio
> > > > > Estevam <fabio.estevam@nxp.com>; shawnguo at kernel.org
> > > > > Subject: Re: [PATCH 4/4] soc: imx: add SC firmware IPC and APIs
> > > > >
> > > > > On Wed, May 02, 2018 at 06:40:03PM +0000, A.s. Dong wrote:
> > > > > > > -----Original Message-----
> > > > > > > From: Sascha Hauer [mailto:s.hauer at pengutronix.de]
> > > > > > > Sent: Wednesday, May 2, 2018 6:04 PM
> > > > > > > To: A.s. Dong <aisheng.dong@nxp.com>
> > > > > > > Cc: linux-arm-kernel at lists.infradead.org;
> > > > > > > dongas86 at gmail.com; dl-linux-imx <linux-imx@nxp.com>;
> > > > > > > kernel at pengutronix.de; Fabio Estevam
> > > > > > > <fabio.estevam@nxp.com>; shawnguo at kernel.org
> > > > > > > Subject: Re: [PATCH 4/4] soc: imx: add SC firmware IPC and
> > > > > > > APIs
> > > > > > >
> > > > > > > On Sat, Apr 28, 2018 at 02:46:16AM +0800, Dong Aisheng wrote:
> > > > > > > > +/* Initialization of the MU code. */ int __init
> > > > > > > > +imx8_scu_init(void) {
> > > > > > > > + struct device_node *np, *mu_np;
> > > > > > > > + struct resource mu_res;
> > > > > > > > + sc_err_t sci_err;
> > > > > > > > + int ret;
> > > > > > > > +
> > > > > > > > + if (!of_machine_is_compatible("fsl,imx8qxp"))
> > > > > > > > + return 0;
> > > > > > > > +
> > > > > > > > + np = of_find_compatible_node(NULL, NULL, "nxp,imx8qxp-
> > > scu");
> > > > > > > > + if (!np)
> > > > > > > > + return -ENODEV;
> > > > > > > > +
> > > > > > > > + mu_np = of_parse_phandle(np, "fsl,mu", 0);
> > > > > > > > + if (WARN_ON(!mu_np))
> > > > > > > > + return -EINVAL;
> > > > > > > > +
> > > > > > > > + ret = of_address_to_resource(mu_np, 0, &mu_res);
> > > > > > > > + if (WARN_ON(ret))
> > > > > > > > + return -EINVAL;
> > > > > > > > +
> > > > > > > > + /* we use mu physical address as IPC communication
> > > > > > > > +channel
> > > ID */
> > > > > > > > + sci_err = sc_ipc_open(&scu_ipc_handle,
> > > (sc_ipc_id_t)(mu_res.start));
> > > > > > > > + if (sci_err != SC_ERR_NONE) {
> > > > > > > > + pr_err("Cannot open MU channel to SCU\n");
> > > > > > > > + return sci_err;
> > > > > > > > + };
> > > > > > >
> > > > > > > Introducing private error codes always has the risk of just
> > > > > > > forwarding them as errno codes as done here.
> > > > > > >
> > > > > >
> > > > > > Yes, you're right.
> > > > > > We probably could do the same as scpi_to_linux_errno in
> > > > > > drivers/firmware/arm_scpi.c.
> > > > > > However, may can't fix the issue permanently.
> > > > > >
> > > > > > > > +
> > > > > > > > + of_node_put(mu_np);
> > > > > > > > +
> > > > > > > > + pr_info("NXP i.MX SCU Initialized (scu_ipc %u)\n",
> > > > > > > > +scu_ipc_handle);
> > > > > > > > +
> > > > > > > > + return 0;
> > > > > > > > +}
> > > > > > > > +early_initcall(imx8_scu_init);
> > > > > > >
> > > > > > > This code shows that the separate 'scu' device node shouldn't
> exist.
> > > > > > > It is only used as a stepping stone to find the 'mu' node.
> > > > > > > Instead of providing a proper driver for the 'mu' node the
> > > > > > > scu code registers it
> > > > > with its physical address.
> > > > > > > I don't think it makes sense to separate mu and scu code in this
> way.
> > > > > > >
> > > > > >
> > > > > > I agree that may not look quite well.
> > > > > > It mainly because we want to use the MU physical address as a MU
> ID.
> > > > > > (can't use virtual address as sc_ipc_id_t is u32 defined by SCU
> firmware.
> > > > > >
> > > > > > If you have any better suggestion please let me know.
> > > > >
> > > > > What I'm suggesting is a single node:
> > > > >
> > > > > scu at 5d1b0000 {
> > > > > compatible = "fsl,imx8qxp-scu";
> > > > > reg = <0x0 0x5d1b0000 0x0 0x10000>;
> > > > > };
> > > > >
> > > > > Attach your code to this one, without any further separation
> > > > > between mu and scu code.
> > > > >
> > > >
> > > > A bit confused. You're suggesting a single node here without mu or
> > > > mailbox node phandle in it? Then how SCU use MU?
> > >
> > > ioremap the address and mu_receive_msg()/mu_send_msg() on it, just
> > > like you do already.
> > >
> > > >
> > > > > We discussed this internally and came to the conclusion that the
> > > > > SCU is not a generic user of a MU. The MU is designed to work
> > > > > together with a piece of SRAM to form a mailbox system. Instead
> > > > > of working as designed the SCU morses the messages through the
> > > > > doorbell (what the MU really is). For anything that uses the MU
> > > > > in the way it is designed I would suggest using the mailbox
> > > > > interface from drivers/mailbox/ along with the binding from
> > > Documentation/devicetree/bindings/mailbox/mailbox.txt.
> > > > >
> > > > > In the way I suggest there is no need for any MU ID.
> > > > >
> > > >
> > > > So you're suggesting switch to use mailbox instead of private MU
> > > > library function calls?
> > > > Something like:
> > > > scu {
> > > > compatible = "nxp,imx8qxp-scu", "simple-bus";
> > > > mboxes = <&mailbox 0>;
> > > > }
> > > > Then IPC is implemented based on mailbox?
> > > >
> > > > As I replied Oleksij Rempel in another mail in this thread, we've
> > > > tried mailbox but got performance regression issue and need more
> > > > time to investigate whether it's really quite suitable for i.MX
> > > > SCU firmware as SCU handling message quite fast in micro seconds.
> > > > (Mailbox polling method has much more latency than current MU
> > > > sample polling we
> > > > used) and we want to avoid the SCU firmware API changes.
> > >
> > > I am not suggesting to do mailboxing (using shared memory) for the SCU.
> > > I am also not suggesting any API update for the SCU communication.
> > > I am just mentioning that doing mailboxing is the way the MU was
> > > originally designed for. Looking at the reference manual I see many MUs
> on the i.MX8.
> > > I guess most of them are for communication between the different
> > > cores on the system. For these it's probably worth writing some generic
> MU driver.
> > > The way the MU is used for communication with the SCU though is so
> > > special that it's not worth writing a generic driver, so just
> > > integrate the MU access functions in the SCU code.
> > >
> >
> > I understand it.
> >
> > One problem of the way you suggested may be that:
> > If we doing like below, we may lose flexibility to change the MU used
> > for SCU firmware communication.
> > scu at 5d1b0000 {
> > compatible = "fsl,imx8qxp-scu";
> > reg = <0x0 0x5d1b0000 0x0 0x10000>;
> > };
> >
> > And current design is that the system supports multiple MU channels
> > used by various users at the same time, e.g. SCU, Power Domain, Clock and
> others.
> > User can flexibly change it under their nodes: And each MU channel is
> > protected by their private lock and not affect each others.
> >
> > e.g.
> > scu {
> > compatible = "nxp,imx8qxp-scu", "simple-bus";
> > fsl,mu = <&lsio_mu0>;
> >
> > clk: clk {
> > compatible = "fsl,imx8qxp-clk";
> > #clock-cells = <1>;
> > };
> >
> > iomuxc: iomuxc {
> > compatible = "fsl,imx8qxp-iomuxc";
> > fsl,mu = <&lsio_mu3>;
> > };
> >
> > imx8qx-pm {
> > #address-cells = <1>;
> > #size-cells = <0>;
> > fsl,mu = <&lsio_mu4>;
> > .............
> > }
> >
> > The default code only uses MU0 which is used by SCU.
> >
> > The change may affect this design. Any ideas?
>
> Sorry for the delay.
>
> You can add the child nodes to the mu nodes they should use:
>
> scu1 {
> compatible = "nxp,imx8qxp-scu";
> reg = <0x0 0x5d1b0000 0x0 0x10000>;
>
> clk: clk {
> compatible = "fsl,imx8qxp-clk";
> #clock-cells = <1>;
> };
>
> ...
> };
>
> scu2 {
> compatible = "nxp,imx8qxp-scu";
> reg = <0x0 someothermu 0x0 0x10000>;
>
> iomuxc: iomuxc {
> compatible = "fsl,imx8qxp-iomuxc";
> };
>
> ...
> };
>
> So instead of adding all possible children to a single mu and phandle to other
> mu's, just add the right children to each mu.
>
I got your point now. But sorry i'm still a bit hestitate to it....
This way increases complexity and looks more like a per-channel binding.
But we normally have only one (abstract) SCU firmware node in a system which may
use different channels to implement different functions like clk, pd and etc.
Multiple faked SCU nodes make people a bit confusing.
Furthermore, it's still lose the flexibility for user to changing a MU to use.
Looking at all exist users in kernel, seems no one to use like this.
See:
Documentation/devicetree/bindings/arm/arm,scpi.txt
Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
All are similar like:
xxx: protocol-node {
compatible = "xxx-protocal";
channel = ...
...
clk_node: clk_node {
...
};
pd_node: pd_node {
...
};
};
The protocol node work is selecting the correct channel, do necessary initialization
and populate the all child function device nodes.
IMHO I'm still a bit like to this common way used in kernel if no stronger objection.
Do you think we can choose this way to go step forward?
Regards
Dong Aisheng
> Sascha
>
>
> --
> Pengutronix e.K. | |
> Industrial Linux Solutions |
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fww
> w.pengutronix.de%2F&data=02%7C01%7Caisheng.dong%40nxp.com%7Cec7
> bf490d73542f736a408d5cbb7e5af%7C686ea1d3bc2b4c6fa92cd99c5c301635%7
> C0%7C0%7C636638913057159824&sdata=m24oKIKeP1sxGOq9gGShs8Y%2BiB
> Rg4NymKKJEF0IHpGc%3D&reserved=0 |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
From: Viresh Kumar @ 2018-06-07 4:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180606142622.2338abf6@vmware.local.home>
+Greg/Alex,
@Fegguang/build-bot: I do see mention of Greg and /me in your initial email's
body saying TO: Viresh, CC: Greg, but I don't see any of us getting cc'd in your
email. Bug ?
On 06-06-18, 14:26, Steven Rostedt wrote:
> On Wed, 6 Jun 2018 16:26:00 +0200
> Johan Hovold <johan@kernel.org> wrote:
>
> > Looks like the greybus code above is working as intended by checking for
> > unterminated string after the strncpy, even if this does now triggers
> > the truncation warning.
So why exactly are we generating a warning here ? Is it because it is possible
that the first n bytes of src may not have the null terminating byte and the
dest may not be null terminated eventually ?
Maybe I should just use memcpy here then ?
But AFAIR, I used strncpy() specifically because it also sets all the remaining
bytes after the null terminating byte with the null terminating byte. And so it
is pretty easy for me to check if the final string is null terminated by
checking [max - 1] byte against '\0', which the code is doing right now.
I am not sure what would the best way to get around this incorrect-warning.
And I am wondering on why buildbot reported the warning only for two instances
in that file, while I have done the same thing at 4 places.
> Ah, yes I now see that. Thanks for pointing it out. But perhaps it
> should also add the "- 1" to the strncpy() so that gcc doesn't think
> it's a mistake.
The src string is passed on from a firmware entity and we need to make sure the
protocol (greybus) is implemented properly by the other end. For example, in the
current case if the firmware sends "HELLOWORLD", its an error as it should have
sent "HELLWORLD\0". But with what you are saying we will forcefully make dest as
"HELLWORLD\0", which wouldn't be the right thing to do as we will miss the bug
present in firmware.
--
viresh
^ permalink raw reply
* [v2, 10/10] dpaa_eth: add the get_ts_info interface for ethtool
From: Yangbo Lu @ 2018-06-07 3:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180607032256.39802-1-yangbo.lu@nxp.com>
Added the get_ts_info interface for ethtool to check
the timestamping capability.
Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
Changes for v2:
- Removed ifdef for hw timestamp.
---
drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c | 39 ++++++++++++++++++++
1 files changed, 39 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
index 2f933b6..3184c8f 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
@@ -32,6 +32,9 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include <linux/string.h>
+#include <linux/of_platform.h>
+#include <linux/net_tstamp.h>
+#include <linux/fsl/ptp_qoriq.h>
#include "dpaa_eth.h"
#include "mac.h"
@@ -515,6 +518,41 @@ static int dpaa_set_rxnfc(struct net_device *dev, struct ethtool_rxnfc *cmd)
return ret;
}
+static int dpaa_get_ts_info(struct net_device *net_dev,
+ struct ethtool_ts_info *info)
+{
+ struct device *dev = net_dev->dev.parent;
+ struct device_node *mac_node = dev->of_node;
+ struct device_node *fman_node = NULL, *ptp_node = NULL;
+ struct platform_device *ptp_dev = NULL;
+ struct qoriq_ptp *ptp = NULL;
+
+ info->phc_index = -1;
+
+ fman_node = of_get_parent(mac_node);
+ if (fman_node)
+ ptp_node = of_parse_phandle(fman_node, "ptimer-handle", 0);
+
+ if (ptp_node)
+ ptp_dev = of_find_device_by_node(ptp_node);
+
+ if (ptp_dev)
+ ptp = platform_get_drvdata(ptp_dev);
+
+ if (ptp)
+ info->phc_index = ptp->phc_index;
+
+ info->so_timestamping = SOF_TIMESTAMPING_TX_HARDWARE |
+ SOF_TIMESTAMPING_RX_HARDWARE |
+ SOF_TIMESTAMPING_RAW_HARDWARE;
+ info->tx_types = (1 << HWTSTAMP_TX_OFF) |
+ (1 << HWTSTAMP_TX_ON);
+ info->rx_filters = (1 << HWTSTAMP_FILTER_NONE) |
+ (1 << HWTSTAMP_FILTER_ALL);
+
+ return 0;
+}
+
const struct ethtool_ops dpaa_ethtool_ops = {
.get_drvinfo = dpaa_get_drvinfo,
.get_msglevel = dpaa_get_msglevel,
@@ -530,4 +568,5 @@ static int dpaa_set_rxnfc(struct net_device *dev, struct ethtool_rxnfc *cmd)
.set_link_ksettings = dpaa_set_link_ksettings,
.get_rxnfc = dpaa_get_rxnfc,
.set_rxnfc = dpaa_set_rxnfc,
+ .get_ts_info = dpaa_get_ts_info,
};
--
1.7.1
^ permalink raw reply related
* [v2, 09/10] dpaa_eth: add support for hardware timestamping
From: Yangbo Lu @ 2018-06-07 3:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180607032256.39802-1-yangbo.lu@nxp.com>
This patch is to add hardware timestamping support
for dpaa_eth. On Rx, timestamping is enabled for
all frames. On Tx, we only instruct the hardware
to timestamp the frames marked accordingly by the
stack.
Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
Changes for v2:
- Removed ifdef for timestamp code.
- Minor fixes for code style.
---
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 101 ++++++++++++++++++++++-
drivers/net/ethernet/freescale/dpaa/dpaa_eth.h | 3 +
2 files changed, 99 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index fd43f98..bd589ac 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -1168,7 +1168,7 @@ static int dpaa_eth_init_tx_port(struct fman_port *port, struct dpaa_fq *errq,
buf_prefix_content.priv_data_size = buf_layout->priv_data_size;
buf_prefix_content.pass_prs_result = true;
buf_prefix_content.pass_hash_result = true;
- buf_prefix_content.pass_time_stamp = false;
+ buf_prefix_content.pass_time_stamp = true;
buf_prefix_content.data_align = DPAA_FD_DATA_ALIGNMENT;
params.specific_params.non_rx_params.err_fqid = errq->fqid;
@@ -1210,7 +1210,7 @@ static int dpaa_eth_init_rx_port(struct fman_port *port, struct dpaa_bp **bps,
buf_prefix_content.priv_data_size = buf_layout->priv_data_size;
buf_prefix_content.pass_prs_result = true;
buf_prefix_content.pass_hash_result = true;
- buf_prefix_content.pass_time_stamp = false;
+ buf_prefix_content.pass_time_stamp = true;
buf_prefix_content.data_align = DPAA_FD_DATA_ALIGNMENT;
rx_p = ¶ms.specific_params.rx_params;
@@ -1592,6 +1592,16 @@ static int dpaa_eth_refill_bpools(struct dpaa_priv *priv)
return 0;
}
+static int dpaa_get_tstamp_ns(struct net_device *net_dev, u64 *ns,
+ struct fman_port *port, const void *data)
+{
+ if (!fman_port_get_tstamp_field(port, data, ns)) {
+ be64_to_cpus(ns);
+ return 0;
+ }
+ return -EINVAL;
+}
+
/* Cleanup function for outgoing frame descriptors that were built on Tx path,
* either contiguous frames or scatter/gather ones.
* Skb freeing is not handled here.
@@ -1607,14 +1617,29 @@ static int dpaa_eth_refill_bpools(struct dpaa_priv *priv)
{
const enum dma_data_direction dma_dir = DMA_TO_DEVICE;
struct device *dev = priv->net_dev->dev.parent;
+ struct skb_shared_hwtstamps shhwtstamps;
dma_addr_t addr = qm_fd_addr(fd);
const struct qm_sg_entry *sgt;
struct sk_buff **skbh, *skb;
int nr_frags, i;
+ u64 ns;
skbh = (struct sk_buff **)phys_to_virt(addr);
skb = *skbh;
+ if (priv->tx_tstamp && skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) {
+ memset(&shhwtstamps, 0, sizeof(shhwtstamps));
+
+ if (!dpaa_get_tstamp_ns(priv->net_dev, &ns,
+ priv->mac_dev->port[TX],
+ (void *)skbh)) {
+ shhwtstamps.hwtstamp = ns_to_ktime(ns);
+ skb_tstamp_tx(skb, &shhwtstamps);
+ } else {
+ dev_warn(dev, "dpaa_get_tstamp_ns failed!\n");
+ }
+ }
+
if (unlikely(qm_fd_get_format(fd) == qm_fd_sg)) {
nr_frags = skb_shinfo(skb)->nr_frags;
dma_unmap_single(dev, addr, qm_fd_get_offset(fd) +
@@ -2086,6 +2111,11 @@ static int dpaa_start_xmit(struct sk_buff *skb, struct net_device *net_dev)
if (unlikely(err < 0))
goto skb_to_fd_failed;
+ if (priv->tx_tstamp && skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) {
+ fd.cmd |= FM_FD_CMD_UPD;
+ skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
+ }
+
if (likely(dpaa_xmit(priv, percpu_stats, queue_mapping, &fd) == 0))
return NETDEV_TX_OK;
@@ -2227,6 +2257,7 @@ static enum qman_cb_dqrr_result rx_default_dqrr(struct qman_portal *portal,
struct qman_fq *fq,
const struct qm_dqrr_entry *dq)
{
+ struct skb_shared_hwtstamps *shhwtstamps;
struct rtnl_link_stats64 *percpu_stats;
struct dpaa_percpu_priv *percpu_priv;
const struct qm_fd *fd = &dq->fd;
@@ -2240,6 +2271,7 @@ static enum qman_cb_dqrr_result rx_default_dqrr(struct qman_portal *portal,
struct sk_buff *skb;
int *count_ptr;
void *vaddr;
+ u64 ns;
fd_status = be32_to_cpu(fd->status);
fd_format = qm_fd_get_format(fd);
@@ -2304,6 +2336,18 @@ static enum qman_cb_dqrr_result rx_default_dqrr(struct qman_portal *portal,
if (!skb)
return qman_cb_dqrr_consume;
+ if (priv->rx_tstamp) {
+ shhwtstamps = skb_hwtstamps(skb);
+ memset(shhwtstamps, 0, sizeof(*shhwtstamps));
+
+ if (!dpaa_get_tstamp_ns(priv->net_dev, &ns,
+ priv->mac_dev->port[RX],
+ vaddr))
+ shhwtstamps->hwtstamp = ns_to_ktime(ns);
+ else
+ dev_warn(net_dev->dev.parent, "dpaa_get_tstamp_ns failed!\n");
+ }
+
skb->protocol = eth_type_trans(skb, net_dev);
if (net_dev->features & NETIF_F_RXHASH && priv->keygen_in_use &&
@@ -2523,11 +2567,58 @@ static int dpaa_eth_stop(struct net_device *net_dev)
return err;
}
+static int dpaa_ts_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
+{
+ struct dpaa_priv *priv = netdev_priv(dev);
+ struct hwtstamp_config config;
+
+ if (copy_from_user(&config, rq->ifr_data, sizeof(config)))
+ return -EFAULT;
+
+ switch (config.tx_type) {
+ case HWTSTAMP_TX_OFF:
+ /* Couldn't disable rx/tx timestamping separately.
+ * Do nothing here.
+ */
+ priv->tx_tstamp = false;
+ break;
+ case HWTSTAMP_TX_ON:
+ priv->mac_dev->set_tstamp(priv->mac_dev->fman_mac, true);
+ priv->tx_tstamp = true;
+ break;
+ default:
+ return -ERANGE;
+ }
+
+ if (config.rx_filter == HWTSTAMP_FILTER_NONE) {
+ /* Couldn't disable rx/tx timestamping separately.
+ * Do nothing here.
+ */
+ priv->rx_tstamp = false;
+ } else {
+ priv->mac_dev->set_tstamp(priv->mac_dev->fman_mac, true);
+ priv->rx_tstamp = true;
+ /* TS is set for all frame types, not only those requested */
+ config.rx_filter = HWTSTAMP_FILTER_ALL;
+ }
+
+ return copy_to_user(rq->ifr_data, &config, sizeof(config)) ?
+ -EFAULT : 0;
+}
+
static int dpaa_ioctl(struct net_device *net_dev, struct ifreq *rq, int cmd)
{
- if (!net_dev->phydev)
- return -EINVAL;
- return phy_mii_ioctl(net_dev->phydev, rq, cmd);
+ int ret = -EINVAL;
+
+ if (cmd == SIOCGMIIREG) {
+ if (net_dev->phydev)
+ return phy_mii_ioctl(net_dev->phydev, rq, cmd);
+ }
+
+ if (cmd == SIOCSHWTSTAMP)
+ return dpaa_ts_ioctl(net_dev, rq, cmd);
+
+ return ret;
}
static const struct net_device_ops dpaa_ops = {
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
index bd94220..af320f8 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
@@ -182,6 +182,9 @@ struct dpaa_priv {
struct dpaa_buffer_layout buf_layout[2];
u16 rx_headroom;
+
+ bool tx_tstamp; /* Tx timestamping enabled */
+ bool rx_tstamp; /* Rx timestamping enabled */
};
/* from dpaa_ethtool.c */
--
1.7.1
^ permalink raw reply related
* [v2, 08/10] fsl/fman: define frame description command UPD
From: Yangbo Lu @ 2018-06-07 3:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180607032256.39802-1-yangbo.lu@nxp.com>
Defined frame description command FM_FD_CMD_UPD for
prepended data updating.
Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
Changes for v2:
- None.
---
drivers/net/ethernet/freescale/fman/fman.h | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fman/fman.h b/drivers/net/ethernet/freescale/fman/fman.h
index bfa02e0..935c317 100644
--- a/drivers/net/ethernet/freescale/fman/fman.h
+++ b/drivers/net/ethernet/freescale/fman/fman.h
@@ -41,6 +41,7 @@
/* Frame queue Context Override */
#define FM_FD_CMD_FCO 0x80000000
#define FM_FD_CMD_RPD 0x40000000 /* Read Prepended Data */
+#define FM_FD_CMD_UPD 0x20000000 /* Update Prepended Data */
#define FM_FD_CMD_DTC 0x10000000 /* Do L4 Checksum */
/* TX-Port: Unsupported Format */
--
1.7.1
^ permalink raw reply related
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