Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 4/6] dmaengine: imx-sdma: remove usless lock
From: Robin Gong @ 2018-06-11 14:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1528729173-28684-1-git-send-email-yibin.gong@nxp.com>

No need anymore for 'lock' now since virtual dma will provide
the common lock instead.

Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
 drivers/dma/imx-sdma.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index e0af8ee..f150b38 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -349,7 +349,6 @@ struct sdma_channel {
 	unsigned long			event_mask[2];
 	unsigned long			watermark_level;
 	u32				shp_addr, per_addr;
-	spinlock_t			lock;
 	enum dma_status			status;
 	struct imx_dma_data		data;
 };
@@ -1907,7 +1906,6 @@ static int sdma_probe(struct platform_device *pdev)
 		struct sdma_channel *sdmac = &sdma->channel[i];
 
 		sdmac->sdma = sdma;
-		spin_lock_init(&sdmac->lock);
 
 		sdmac->channel = i;
 		sdmac->vc.desc_free = sdma_desc_free;
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 5/6] dmaengine: imx-sdma: remove the maximum limation for bd numbers
From: Robin Gong @ 2018-06-11 14:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1528729173-28684-1-git-send-email-yibin.gong@nxp.com>

No this limitation now after virtual dma used since bd is allocated
dynamically instead of static.

Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
 drivers/dma/imx-sdma.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index f150b38..0b0588d2 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -292,7 +292,6 @@ struct sdma_context_data {
 	u32  scratch7;
 } __attribute__ ((packed));
 
-#define NUM_BD (int)(PAGE_SIZE / sizeof(struct sdma_buffer_descriptor))
 
 struct sdma_engine;
 
@@ -1296,13 +1295,6 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
 	if (ret)
 		goto err_bd_out;
 
-	if (sg_len > NUM_BD) {
-		dev_err(sdma->dev, "SDMA channel %d: maximum number of sg exceeded: %d > %d\n",
-				channel, sg_len, NUM_BD);
-		ret = -EINVAL;
-		goto err_bd_out;
-	}
-
 	desc->chn_count = 0;
 	for_each_sg(sgl, sg, sg_len, i) {
 		struct sdma_buffer_descriptor *bd = &desc->bd[i];
@@ -1412,12 +1404,6 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
 	if (ret)
 		goto err_bd_out;
 
-	if (num_periods > NUM_BD) {
-		dev_err(sdma->dev, "SDMA channel %d: maximum number of sg exceeded: %d > %d\n",
-				channel, num_periods, NUM_BD);
-		goto err_bd_out;
-	}
-
 	if (period_len > 0xffff) {
 		dev_err(sdma->dev, "SDMA channel %d: maximum period size exceeded: %zu > %d\n",
 				channel, period_len, 0xffff);
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 6/6] dmaengine: imx-sdma: add sdma_transfer_init to decrease code overlap
From: Robin Gong @ 2018-06-11 14:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1528729173-28684-1-git-send-email-yibin.gong@nxp.com>

There are lot of codes overlap between prep_sg and prep_cyclic function.
Add sdma_transfer_init() function to elimated the code overlap.

Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
 drivers/dma/imx-sdma.c | 83 ++++++++++++++++++++++----------------------------
 1 file changed, 37 insertions(+), 46 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 0b0588d2..486ebfe 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -1254,6 +1254,40 @@ static void sdma_free_chan_resources(struct dma_chan *chan)
 	clk_disable(sdma->clk_ahb);
 }
 
+static struct sdma_desc *sdma_transfer_init(struct sdma_channel *sdmac,
+				enum dma_transfer_direction direction, u32 bds)
+{
+	struct sdma_desc *desc;
+
+	desc = kzalloc((sizeof(*desc)), GFP_KERNEL);
+	if (!desc)
+		goto err_out;
+
+	sdmac->status = DMA_IN_PROGRESS;
+	sdmac->direction = direction;
+	sdmac->flags = 0;
+
+	desc->chn_count = 0;
+	desc->chn_real_count = 0;
+	desc->buf_tail = 0;
+	desc->buf_ptail = 0;
+	desc->sdmac = sdmac;
+	desc->num_bd = bds;
+
+	if (sdma_alloc_bd(desc))
+		goto err_desc_out;
+
+	if (sdma_load_context(sdmac))
+		goto err_desc_out;
+
+	return desc;
+
+err_desc_out:
+	kfree(desc);
+err_out:
+	return NULL;
+}
+
 static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
 		struct dma_chan *chan, struct scatterlist *sgl,
 		unsigned int sg_len, enum dma_transfer_direction direction,
@@ -1266,36 +1300,13 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
 	struct scatterlist *sg;
 	struct sdma_desc *desc;
 
-	if (sdmac->status == DMA_IN_PROGRESS)
-		return NULL;
-	sdmac->status = DMA_IN_PROGRESS;
-
-	sdmac->flags = 0;
-
-	desc = kzalloc((sizeof(*desc)), GFP_KERNEL);
+	desc = sdma_transfer_init(sdmac, direction, sg_len);
 	if (!desc)
 		goto err_out;
 
-	desc->buf_tail = 0;
-	desc->buf_ptail = 0;
-	desc->sdmac = sdmac;
-	desc->num_bd = sg_len;
-	desc->chn_real_count = 0;
-
-	if (sdma_alloc_bd(desc)) {
-		kfree(desc);
-		goto err_out;
-	}
-
 	dev_dbg(sdma->dev, "setting up %d entries for channel %d.\n",
 			sg_len, channel);
 
-	sdmac->direction = direction;
-	ret = sdma_load_context(sdmac);
-	if (ret)
-		goto err_bd_out;
-
-	desc->chn_count = 0;
 	for_each_sg(sgl, sg, sg_len, i) {
 		struct sdma_buffer_descriptor *bd = &desc->bd[i];
 		int param;
@@ -1371,38 +1382,18 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
 	struct sdma_engine *sdma = sdmac->sdma;
 	int num_periods = buf_len / period_len;
 	int channel = sdmac->channel;
-	int ret, i = 0, buf = 0;
+	int i = 0, buf = 0;
 	struct sdma_desc *desc;
 
 	dev_dbg(sdma->dev, "%s channel: %d\n", __func__, channel);
 
-	if (sdmac->status == DMA_IN_PROGRESS)
-		return NULL;
-
-	sdmac->status = DMA_IN_PROGRESS;
-
-	desc = kzalloc((sizeof(*desc)), GFP_KERNEL);
+	desc = sdma_transfer_init(sdmac, direction, num_periods);
 	if (!desc)
 		goto err_out;
 
-	desc->buf_tail = 0;
-	desc->buf_ptail = 0;
-	desc->sdmac = sdmac;
-	desc->num_bd = num_periods;
-	desc->chn_real_count = 0;
 	desc->period_len = period_len;
 
 	sdmac->flags |= IMX_DMA_SG_LOOP;
-	sdmac->direction = direction;
-
-	if (sdma_alloc_bd(desc)) {
-		kfree(desc);
-		goto err_bd_out;
-	}
-
-	ret = sdma_load_context(sdmac);
-	if (ret)
-		goto err_bd_out;
 
 	if (period_len > 0xffff) {
 		dev_err(sdma->dev, "SDMA channel %d: maximum period size exceeded: %zu > %d\n",
-- 
2.7.4

^ permalink raw reply related

* [PATCH v2 0/1] ASoC: stm32: sai: add iec958 controls support
From: Olivier Moysan @ 2018-06-11 15:13 UTC (permalink / raw)
  To: linux-arm-kernel

Changes v2:
- Remove iec958 helpers and implement iec958 controls in sai driver 

Olivier Moysan (1):
  ASoC: stm32: sai: add iec958 controls support

 sound/soc/stm/Kconfig         |   1 +
 sound/soc/stm/stm32_sai_sub.c | 139 ++++++++++++++++++++++++++++++++++++++----
 2 files changed, 128 insertions(+), 12 deletions(-)

-- 
1.9.1

^ permalink raw reply

* [PATCH v2 1/1] ASoC: stm32: sai: add iec958 controls support
From: Olivier Moysan @ 2018-06-11 15:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1528730039-15757-1-git-send-email-olivier.moysan@st.com>

Add support of iec958 controls for STM32 SAI.

Signed-off-by: Olivier Moysan <olivier.moysan@st.com>
---
 sound/soc/stm/Kconfig         |   1 +
 sound/soc/stm/stm32_sai_sub.c | 139 ++++++++++++++++++++++++++++++++++++++----
 2 files changed, 128 insertions(+), 12 deletions(-)

diff --git a/sound/soc/stm/Kconfig b/sound/soc/stm/Kconfig
index 48f9ddd94016..9b2681397dba 100644
--- a/sound/soc/stm/Kconfig
+++ b/sound/soc/stm/Kconfig
@@ -6,6 +6,7 @@ config SND_SOC_STM32_SAI
 	depends on SND_SOC
 	select SND_SOC_GENERIC_DMAENGINE_PCM
 	select REGMAP_MMIO
+	select SND_PCM_IEC958
 	help
 	  Say Y if you want to enable SAI for STM32
 
diff --git a/sound/soc/stm/stm32_sai_sub.c b/sound/soc/stm/stm32_sai_sub.c
index cfeb219e1d78..c4f15ea14197 100644
--- a/sound/soc/stm/stm32_sai_sub.c
+++ b/sound/soc/stm/stm32_sai_sub.c
@@ -96,7 +96,8 @@
  * @slot_mask: rx or tx active slots mask. set at init or at runtime
  * @data_size: PCM data width. corresponds to PCM substream width.
  * @spdif_frm_cnt: S/PDIF playback frame counter
- * @spdif_status_bits: S/PDIF status bits
+ * @snd_aes_iec958: iec958 data
+ * @ctrl_lock: control lock
  */
 struct stm32_sai_sub_data {
 	struct platform_device *pdev;
@@ -125,7 +126,8 @@ struct stm32_sai_sub_data {
 	int slot_mask;
 	int data_size;
 	unsigned int spdif_frm_cnt;
-	unsigned char spdif_status_bits[SAI_IEC60958_STATUS_BYTES];
+	struct snd_aes_iec958 iec958;
+	struct mutex ctrl_lock; /* protect resources accessed by controls */
 };
 
 enum stm32_sai_fifo_th {
@@ -184,10 +186,6 @@ static bool stm32_sai_sub_writeable_reg(struct device *dev, unsigned int reg)
 	}
 }
 
-static const unsigned char default_status_bits[SAI_IEC60958_STATUS_BYTES] = {
-	0, 0, 0, IEC958_AES3_CON_FS_48000,
-};
-
 static const struct regmap_config stm32_sai_sub_regmap_config_f4 = {
 	.reg_bits = 32,
 	.reg_stride = 4,
@@ -210,6 +208,49 @@ static bool stm32_sai_sub_writeable_reg(struct device *dev, unsigned int reg)
 	.fast_io = true,
 };
 
+static int snd_pcm_iec958_info(struct snd_kcontrol *kcontrol,
+			       struct snd_ctl_elem_info *uinfo)
+{
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_IEC958;
+	uinfo->count = 1;
+
+	return 0;
+}
+
+static int snd_pcm_iec958_get(struct snd_kcontrol *kcontrol,
+			      struct snd_ctl_elem_value *uctl)
+{
+	struct stm32_sai_sub_data *sai = snd_kcontrol_chip(kcontrol);
+
+	mutex_lock(&sai->ctrl_lock);
+	memcpy(uctl->value.iec958.status, sai->iec958.status, 4);
+	mutex_unlock(&sai->ctrl_lock);
+
+	return 0;
+}
+
+static int snd_pcm_iec958_put(struct snd_kcontrol *kcontrol,
+			      struct snd_ctl_elem_value *uctl)
+{
+	struct stm32_sai_sub_data *sai = snd_kcontrol_chip(kcontrol);
+
+	mutex_lock(&sai->ctrl_lock);
+	memcpy(sai->iec958.status, uctl->value.iec958.status, 4);
+	mutex_unlock(&sai->ctrl_lock);
+
+	return 0;
+}
+
+static const struct snd_kcontrol_new iec958_ctls = {
+	.access = (SNDRV_CTL_ELEM_ACCESS_READWRITE |
+			SNDRV_CTL_ELEM_ACCESS_VOLATILE),
+	.iface = SNDRV_CTL_ELEM_IFACE_PCM,
+	.name = SNDRV_CTL_NAME_IEC958("", PLAYBACK, DEFAULT),
+	.info = snd_pcm_iec958_info,
+	.get = snd_pcm_iec958_get,
+	.put = snd_pcm_iec958_put,
+};
+
 static irqreturn_t stm32_sai_isr(int irq, void *devid)
 {
 	struct stm32_sai_sub_data *sai = (struct stm32_sai_sub_data *)devid;
@@ -619,6 +660,59 @@ static void stm32_sai_set_frame(struct snd_soc_dai *cpu_dai)
 	}
 }
 
+static void stm32_sai_init_iec958_status(struct stm32_sai_sub_data *sai)
+{
+	unsigned char *cs = sai->iec958.status;
+
+	cs[0] = IEC958_AES0_CON_NOT_COPYRIGHT | IEC958_AES0_CON_EMPHASIS_NONE;
+	cs[1] = IEC958_AES1_CON_GENERAL;
+	cs[2] = IEC958_AES2_CON_SOURCE_UNSPEC | IEC958_AES2_CON_CHANNEL_UNSPEC;
+	cs[3] = IEC958_AES3_CON_CLOCK_1000PPM | IEC958_AES3_CON_FS_NOTID;
+}
+
+static void stm32_sai_set_iec958_status(struct stm32_sai_sub_data *sai,
+					struct snd_pcm_runtime *runtime)
+{
+	if (!runtime)
+		return;
+
+	/* Force the sample rate according to runtime rate */
+	mutex_lock(&sai->ctrl_lock);
+	switch (runtime->rate) {
+	case 22050:
+		sai->iec958.status[3] = IEC958_AES3_CON_FS_22050;
+		break;
+	case 44100:
+		sai->iec958.status[3] = IEC958_AES3_CON_FS_44100;
+		break;
+	case 88200:
+		sai->iec958.status[3] = IEC958_AES3_CON_FS_88200;
+		break;
+	case 176400:
+		sai->iec958.status[3] = IEC958_AES3_CON_FS_176400;
+		break;
+	case 24000:
+		sai->iec958.status[3] = IEC958_AES3_CON_FS_24000;
+		break;
+	case 48000:
+		sai->iec958.status[3] = IEC958_AES3_CON_FS_48000;
+		break;
+	case 96000:
+		sai->iec958.status[3] = IEC958_AES3_CON_FS_96000;
+		break;
+	case 192000:
+		sai->iec958.status[3] = IEC958_AES3_CON_FS_192000;
+		break;
+	case 32000:
+		sai->iec958.status[3] = IEC958_AES3_CON_FS_32000;
+		break;
+	default:
+		sai->iec958.status[3] = IEC958_AES3_CON_FS_NOTID;
+		break;
+	}
+	mutex_unlock(&sai->ctrl_lock);
+}
+
 static int stm32_sai_configure_clock(struct snd_soc_dai *cpu_dai,
 				     struct snd_pcm_hw_params *params)
 {
@@ -709,7 +803,11 @@ static int stm32_sai_hw_params(struct snd_pcm_substream *substream,
 
 	sai->data_size = params_width(params);
 
-	if (!STM_SAI_PROTOCOL_IS_SPDIF(sai)) {
+	if (STM_SAI_PROTOCOL_IS_SPDIF(sai)) {
+		/* Rate not already set in runtime structure */
+		substream->runtime->rate = params_rate(params);
+		stm32_sai_set_iec958_status(sai, substream->runtime);
+	} else {
 		ret = stm32_sai_set_slots(cpu_dai);
 		if (ret < 0)
 			return ret;
@@ -789,6 +887,20 @@ static void stm32_sai_shutdown(struct snd_pcm_substream *substream,
 	sai->substream = NULL;
 }
 
+static int stm32_sai_pcm_new(struct snd_soc_pcm_runtime *rtd,
+			     struct snd_soc_dai *cpu_dai)
+{
+	struct stm32_sai_sub_data *sai = dev_get_drvdata(cpu_dai->dev);
+
+	if (STM_SAI_PROTOCOL_IS_SPDIF(sai)) {
+		dev_dbg(&sai->pdev->dev, "%s: register iec controls", __func__);
+		return snd_ctl_add(rtd->pcm->card,
+				   snd_ctl_new1(&iec958_ctls, sai));
+	}
+
+	return 0;
+}
+
 static int stm32_sai_dai_probe(struct snd_soc_dai *cpu_dai)
 {
 	struct stm32_sai_sub_data *sai = dev_get_drvdata(cpu_dai->dev);
@@ -809,6 +921,10 @@ static int stm32_sai_dai_probe(struct snd_soc_dai *cpu_dai)
 	else
 		snd_soc_dai_init_dma_data(cpu_dai, NULL, &sai->dma_params);
 
+	/* Next settings are not relevant for spdif mode */
+	if (STM_SAI_PROTOCOL_IS_SPDIF(sai))
+		return 0;
+
 	cr1_mask = SAI_XCR1_RX_TX;
 	if (STM_SAI_IS_CAPTURE(sai))
 		cr1 |= SAI_XCR1_RX_TX;
@@ -820,10 +936,6 @@ static int stm32_sai_dai_probe(struct snd_soc_dai *cpu_dai)
 				     sai->synco, sai->synci);
 	}
 
-	if (STM_SAI_PROTOCOL_IS_SPDIF(sai))
-		memcpy(sai->spdif_status_bits, default_status_bits,
-		       sizeof(default_status_bits));
-
 	cr1_mask |= SAI_XCR1_SYNCEN_MASK;
 	cr1 |= SAI_XCR1_SYNCEN_SET(sai->sync);
 
@@ -861,7 +973,7 @@ static int stm32_sai_pcm_process_spdif(struct snd_pcm_substream *substream,
 		/* Set channel status bit */
 		byte = frm_cnt >> 3;
 		mask = 1 << (frm_cnt - (byte << 3));
-		if (sai->spdif_status_bits[byte] & mask)
+		if (sai->iec958.status[byte] & mask)
 			*ptr |= 0x04000000;
 		ptr++;
 
@@ -888,6 +1000,7 @@ static int stm32_sai_pcm_process_spdif(struct snd_pcm_substream *substream,
 static struct snd_soc_dai_driver stm32_sai_playback_dai[] = {
 {
 		.probe = stm32_sai_dai_probe,
+		.pcm_new = stm32_sai_pcm_new,
 		.id = 1, /* avoid call to fmt_single_name() */
 		.playback = {
 			.channels_min = 1,
@@ -998,6 +1111,7 @@ static int stm32_sai_sub_parse_of(struct platform_device *pdev,
 			dev_err(&pdev->dev, "S/PDIF IEC60958 not supported\n");
 			return -EINVAL;
 		}
+		stm32_sai_init_iec958_status(sai);
 		sai->spdif = true;
 		sai->master = true;
 	}
@@ -1114,6 +1228,7 @@ static int stm32_sai_sub_probe(struct platform_device *pdev)
 	sai->id = (uintptr_t)of_id->data;
 
 	sai->pdev = pdev;
+	mutex_init(&sai->ctrl_lock);
 	platform_set_drvdata(pdev, sai);
 
 	sai->pdata = dev_get_drvdata(pdev->dev.parent);
-- 
1.9.1

^ permalink raw reply related

* [PATCH V3] ARM: shmobile: Rework the PMIC IRQ line quirk
From: Marek Vasut @ 2018-06-11 15:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAMuHMdUSmpkwNUV+x9oGjsJdN42XBuUBNd0vSn=bgyX+953YOw@mail.gmail.com>

On 06/11/2018 04:30 PM, Geert Uytterhoeven wrote:
> Hi Marek,
> 
> On Mon, Jun 11, 2018 at 4:19 PM Marek Vasut <marek.vasut@gmail.com> wrote:
>> On 06/11/2018 04:10 PM, Geert Uytterhoeven wrote:
>>> On Mon, Jun 11, 2018 at 4:04 PM Marek Vasut <marek.vasut@gmail.com> wrote:
>>>> On 06/11/2018 03:49 PM, Geert Uytterhoeven wrote:
>>>>> On Mon, Jun 11, 2018 at 3:39 PM Marek Vasut <marek.vasut@gmail.com> wrote:
>>>>>> On 06/11/2018 03:03 PM, Geert Uytterhoeven wrote:
>>>>>>> On Mon, Jun 11, 2018 at 2:15 PM Marek Vasut <marek.vasut@gmail.com> wrote:
>>>>>>>> On 06/11/2018 11:56 AM, Geert Uytterhoeven wrote:
>>>>>>>>> On Mon, Jun 4, 2018 at 7:59 PM Marek Vasut <marek.vasut@gmail.com> wrote:
>>>>>>>>>> Rather than hard-coding the quirk topology, which stopped scaling,
>>>>>>>>>> parse the information from DT. The code looks for all compatible
>>>>>>>>>> PMICs -- da9036 and da9210 -- and checks if their IRQ line is tied
>>>>>>>>>> to the same pin. If so, the code sends a matching sequence to the
>>>>>>>>>> PMIC to deassert the IRQ.
>>>>>
>>>>>>>>>> +               ret = of_property_read_u32(np, "reg", &addr);
>>>>>>>>>> +               if (ret)
>>>>>>>>>> +                       return ret;
>>>>>>>>>
>>>>>>>>> I think it's safer to skip this entry and continue, after calling
>>>>>>>>> kfree(quirk), of course.
>>>>>>>>>
>>>>>>>>>> +
>>>>>>>>>> +               quirk->id = id;
>>>>>>>>>> +               quirk->i2c_msg.addr = addr;
>>>>>>>>>> +               quirk->shared = false;
>>>>>>>>>> +
>>>>>>>>>> +               ret = of_irq_parse_one(np, 0, &quirk->irq_args);
>>>>>>>>>> +               if (ret)
>>>>>>>>>> +                       return ret;
>>>>>>>>>
>>>>>>>>> kfree(quirk) and continue...
>>>>>>>>
>>>>>>>> I wonder if it shouldn't rather free the entire list and abort ?
>>>>>>>
>>>>>>> "Be strict when sending, be liberal when receiving."
>>>>>>
>>>>>> Meaning ? I think "the language barrier is protecting me" (TM)
>>>>>
>>>>> Do the best you can, given the buggy DT you received.
>>>>> I.e. don't fail completely, just ignore the bad device node, and continue.
>>>>
>>>> But if you ignore node, you might as well ignore one which is shared and
>>>> then the system crashes due to IRQ storm anyway. So hum, what can we do ?
>>>
>>> Correct. If it's a critical node, it will crash regardless.
>>> If it's a non-critical node, you have the choice between aborting and crashing,
>>> or ignoring and keeping the system alive. Your call.
>>
>> But wait, since we control which machines this code runs on , can't we
>> assure they have valid DTs ? This situation with invalid DT starts to
>> look a bit hypothetical to me.
> 
> That assumes you keep the list of machines to check, and don't want to fix the
> issue automatically when detected (on any R-Car Gen2 or RZ/G1 platform, so
> you still need to check for r8a779[0-4] and r8a774[23457]).

Yes, I want to keep a list of machines to check, to be _sure_ some
machine doesn't randomly blow up.

> Anyway, as we care about booting old DTBs on new kernels (for a while), we
> have a few more release cycles to bikeshed ;-)

I was about to ask if this patch then makes any sense or not.

-- 
Best regards,
Marek Vasut

^ permalink raw reply

* [PATCH v6 3/6] mfd: at91-usart: added mfd driver for usart
From: Alexandre Belloni @ 2018-06-11 15:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180607110020.20565-4-radu.pirea@microchip.com>

On 07/06/2018 14:00:17+0300, Radu Pirea wrote:
> +static int at91_usart_mode_probe(struct platform_device *pdev)
> +{
> +	struct mfd_cell cell;
> +	u32 opmode;

This has to be initialized to AT91_USART_MODE_SERIAL...

> +	int err;
> +
> +	err = device_property_read_u32(&pdev->dev, "atmel,usart-mode", &opmode);
> +

because if the property is not present, then opmode will not be modified
which means it could hold the value of AT91_USART_MODE_SPI.


-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* [PATCH v6 2/6] dt-bindings: add binding for atmel-usart in SPI mode
From: Alexandre Belloni @ 2018-06-11 15:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180607110020.20565-3-radu.pirea@microchip.com>

Hi,

On 07/06/2018 14:00:16+0300, Radu Pirea wrote:
> diff --git a/include/dt-bindings/mfd/at91-usart.h b/include/dt-bindings/mfd/at91-usart.h
> new file mode 100644
> index 000000000000..ac811628a42d
> --- /dev/null
> +++ b/include/dt-bindings/mfd/at91-usart.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * This header provides macros for AT91 USART DT bindings.
> + *
> + * Copyright (C) 2018 Microchip Technology
> + *
> + * Author: Radu Pirea <radu.pirea@microchip.com>
> + *
> + */
> +
> +#ifndef __DT_BINDINGS_AT91_USART_H__
> +#define __DT_BINDINGS_AT91_USART_H__
> +
> +#define AT91_USART_MODE_SERIAL	1
> +#define AT91_USART_MODE_SPI	2
> +

Nitpick, I'd make that 0 and 1 instead of 1 and 2. That would feel more
natural to me regarding the default value.


-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

^ 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-11 15:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180608160355.67712eb8@vmware.local.home>

On Fri, Jun 08, 2018 at 04:03:55PM -0400, Steven Rostedt wrote:
> On Thu, 7 Jun 2018 11:18:16 +0200
> Johan Hovold <johan@kernel.org> wrote:
> 
> 
> > If you want to work around the warning and think you can do it in some
> > non-contrived way, then go for it.
> > 
> > Clearing the request buffer, checking for termination using strnlen, and
> > then using memcpy might not be too bad.
> > 
> > But after all, it is a false positive, so leaving things as they stand
> > is fine too.
> 
> Not sure how contrived you think this is, but it solves the warning
> without adding extra work in the normal case.
> 
> -- Steve
> 
> diff --git a/drivers/staging/greybus/fw-management.c b/drivers/staging/greybus/fw-management.c
> index 71aec14f8181..4fb9f1dff47d 100644
> --- a/drivers/staging/greybus/fw-management.c
> +++ b/drivers/staging/greybus/fw-management.c
> @@ -150,15 +150,18 @@ static int fw_mgmt_load_and_validate_operation(struct fw_mgmt *fw_mgmt,
>  	}
>  
>  	request.load_method = load_method;
> -	strncpy(request.firmware_tag, tag, GB_FIRMWARE_TAG_MAX_SIZE);
> +	strncpy(request.firmware_tag, tag, GB_FIRMWARE_TAG_MAX_SIZE - 1);
>  
>  	/*
>  	 * The firmware-tag should be NULL terminated, otherwise throw error and
>  	 * fail.
>  	 */
> -	if (request.firmware_tag[GB_FIRMWARE_TAG_MAX_SIZE - 1] != '\0') {
> -		dev_err(fw_mgmt->parent, "load-and-validate: firmware-tag is not NULL terminated\n");
> -		return -EINVAL;
> +	if (request.firmware_tag[GB_FIRMWARE_TAG_MAX_SIZE - 2] != '\0') {
> +		if (tag[GB_FIRMWARE_TAG_MAX_SIZE - 1] != '\0') {
> +			dev_err(fw_mgmt->parent, "load-and-validate: firmware-tag is not NULL terminated\n");
> +			return -EINVAL;
> +		}
> +		request.firmware_tag[GB_FIRMWARE_TAG_MAX_SIZE - 1] = '\0';
>  	}

Well, I think it's quite far from obvious what is going on above, and
not least why things are being done this way (which a comment may help
with).

And just NUL-terminating the (automatic) buffer before returning wasn't
enough? Then it may be better to do away with strncpy completely.

But should we really be working around gcc this way? If the
implementation of this new warning isn't smart enough yet, should it not
just be disabled instead?

Thanks,
Johan

^ permalink raw reply

* [PATCH v7 0/5] Reintroduce i.MX EPIT Timer
From: Clément Péron @ 2018-06-11 15:49 UTC (permalink / raw)
  To: linux-arm-kernel

From: Cl?ment Peron <clement.peron@devialet.com>

As suggested in the commit message we have added the device tree support,
proper bindings and we moved the driver into the correct folder.

Moreover we made some changes like use of relaxed IO accesor,
implement sched_clock, delay_timer and reduce the clockevents min_delta.

Changes since v6:
-fix indent

Changes since v5:
- change epit to timer in doc example
- fix typo in imx6sl.dtsi

Changes since v4:
- removed ipg clk
- change in dt epit to timer
- add introduction in doc
- add all compatibles in doc
- update epit entry for other i.MX device-trees

Changes since v3:
- Clean Kconfig
- Rename imx6q-epit to imx31-epit
- Update doc and bindings
- Indent and fix

Changes since v2 (Thanks Fabio Estevam):
- Removed unused ckil clock
- Add out_iounmap
- Check and handle if clk_prepare_enable failed
- Fix comment typo

Changes since v1 (Thanks Vladimir Zapolskiy):
- Add OF dependency in Kconfig
- Sort header
- Use BIT macro
- Remove useless comments
- Fix incorrect indent
- Fix memory leak
- Add check and handle possible returned error

Cl?ment Peron (2):
  ARM: imx: remove inexistant EPIT timer init
  dt-bindings: timer: add i.MX EPIT timer binding

Colin Didier (3):
  clk: imx6: add EPIT clock support
  clocksource: add driver for i.MX EPIT timer
  ARM: dts: imx: add missing compatible and clock properties for EPIT

 .../devicetree/bindings/timer/fsl,imxepit.txt |  21 ++
 arch/arm/boot/dts/imx25.dtsi                  |   8 +-
 arch/arm/boot/dts/imx6qdl.dtsi                |  10 +-
 arch/arm/boot/dts/imx6sl.dtsi                 |  10 +-
 arch/arm/boot/dts/imx6sx.dtsi                 |  10 +-
 arch/arm/boot/dts/imx6ul.dtsi                 |  10 +-
 arch/arm/mach-imx/common.h                    |   1 -
 drivers/clk/imx/clk-imx6q.c                   |   2 +
 drivers/clocksource/Kconfig                   |  11 +
 drivers/clocksource/Makefile                  |   1 +
 drivers/clocksource/timer-imx-epit.c          | 265 ++++++++++++++++++
 include/dt-bindings/clock/imx6qdl-clock.h     |   4 +-
 12 files changed, 341 insertions(+), 12 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/timer/fsl,imxepit.txt
 create mode 100644 drivers/clocksource/timer-imx-epit.c

-- 
2.17.1

^ permalink raw reply

* [PATCH v7 1/5] clk: imx6: add EPIT clock support
From: Clément Péron @ 2018-06-11 15:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180611155001.3506-1-peron.clem@gmail.com>

From: Colin Didier <colin.didier@devialet.com>

Ignore this Patch, already merged in clk-next

Signed-off-by: Colin Didier <colin.didier@devialet.com>
Signed-off-by: Cl?ment Peron <clement.peron@devialet.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 drivers/clk/imx/clk-imx6q.c               | 2 ++
 include/dt-bindings/clock/imx6qdl-clock.h | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c
index 8d518ad5dc13..b9ea7037e193 100644
--- a/drivers/clk/imx/clk-imx6q.c
+++ b/drivers/clk/imx/clk-imx6q.c
@@ -753,6 +753,8 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
 	else
 		clk[IMX6Q_CLK_ECSPI5] = imx_clk_gate2("ecspi5",        "ecspi_root",        base + 0x6c, 8);
 	clk[IMX6QDL_CLK_ENET]         = imx_clk_gate2("enet",          "ipg",               base + 0x6c, 10);
+	clk[IMX6QDL_CLK_EPIT1]        = imx_clk_gate2("epit1",         "ipg",               base + 0x6c, 12);
+	clk[IMX6QDL_CLK_EPIT2]        = imx_clk_gate2("epit2",         "ipg",               base + 0x6c, 14);
 	clk[IMX6QDL_CLK_ESAI_EXTAL]   = imx_clk_gate2_shared("esai_extal",   "esai_podf",   base + 0x6c, 16, &share_count_esai);
 	clk[IMX6QDL_CLK_ESAI_IPG]     = imx_clk_gate2_shared("esai_ipg",   "ahb",           base + 0x6c, 16, &share_count_esai);
 	clk[IMX6QDL_CLK_ESAI_MEM]     = imx_clk_gate2_shared("esai_mem", "ahb",             base + 0x6c, 16, &share_count_esai);
diff --git a/include/dt-bindings/clock/imx6qdl-clock.h b/include/dt-bindings/clock/imx6qdl-clock.h
index da59fd9cdb5e..7ad171b8f3bf 100644
--- a/include/dt-bindings/clock/imx6qdl-clock.h
+++ b/include/dt-bindings/clock/imx6qdl-clock.h
@@ -271,6 +271,8 @@
 #define IMX6QDL_CLK_PRE_AXI			258
 #define IMX6QDL_CLK_MLB_SEL			259
 #define IMX6QDL_CLK_MLB_PODF			260
-#define IMX6QDL_CLK_END				261
+#define IMX6QDL_CLK_EPIT1			261
+#define IMX6QDL_CLK_EPIT2			262
+#define IMX6QDL_CLK_END				263
 
 #endif /* __DT_BINDINGS_CLOCK_IMX6QDL_H */
-- 
2.17.1

^ permalink raw reply related

* [PATCH v7 2/5] ARM: imx: remove inexistant EPIT timer init
From: Clément Péron @ 2018-06-11 15:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180611155001.3506-1-peron.clem@gmail.com>

From: Cl?ment Peron <clement.peron@devialet.com>

i.MX EPIT timer has been removed but not the init function declaration.

Signed-off-by: Cl?ment Peron <clement.peron@devialet.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>
Tested-by: Vladimir Zapolskiy <vz@mleia.com>
---
 arch/arm/mach-imx/common.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h
index c8d68e918b2f..18aae76fa2da 100644
--- a/arch/arm/mach-imx/common.h
+++ b/arch/arm/mach-imx/common.h
@@ -38,7 +38,6 @@ void imx21_soc_init(void);
 void imx27_soc_init(void);
 void imx31_soc_init(void);
 void imx35_soc_init(void);
-void epit_timer_init(void __iomem *base, int irq);
 int mx21_clocks_init(unsigned long lref, unsigned long fref);
 int mx27_clocks_init(unsigned long fref);
 int mx31_clocks_init(unsigned long fref);
-- 
2.17.1

^ permalink raw reply related

* [PATCH v7 3/5] dt-bindings: timer: add i.MX EPIT timer binding
From: Clément Péron @ 2018-06-11 15:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180611155001.3506-1-peron.clem@gmail.com>

From: Cl?ment Peron <clement.peron@devialet.com>

Add devicetree binding document for NXP's i.MX SoC specific
EPIT timer driver.

Signed-off-by: Cl?ment Peron <clement.peron@devialet.com>
Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>
---
 .../devicetree/bindings/timer/fsl,imxepit.txt | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/fsl,imxepit.txt

diff --git a/Documentation/devicetree/bindings/timer/fsl,imxepit.txt b/Documentation/devicetree/bindings/timer/fsl,imxepit.txt
new file mode 100644
index 000000000000..819d6458a860
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/fsl,imxepit.txt
@@ -0,0 +1,21 @@
+Binding for the i.MX Enhanced Periodic Interrupt Timer (EPIT)
+
+The Enhanced Periodic Interrupt Timer (EPIT) is a 32-bit set-and-forget timer
+that is capable of providing precise interrupts at regular intervals with
+minimal processor intervention.
+
+Required properties:
+- compatible: should be "fsl,<chip>-epit", "fsl,imx31-epit" where <chip> is
+  imx25, imx6qdl, imx6sl, imx6sul or imx6sx.
+- reg: physical base address of the controller and length of memory mapped
+  region.
+- interrupts: Should contain EPIT controller interrupt
+- clocks : The clock provided by the SoC to drive the timer.
+
+Example for i.MX6QDL:
+	epit1: timer at 20d0000 {
+		compatible = "fsl,imx6qdl-epit", "fsl,imx31-epit";
+		reg = <0x020d0000 0x4000>;
+		interrupts = <0 56 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&clks IMX6QDL_CLK_EPIT1>;
+	};
-- 
2.17.1

^ permalink raw reply related

* [PATCH v7 4/5] clocksource: add driver for i.MX EPIT timer
From: Clément Péron @ 2018-06-11 15:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180611155001.3506-1-peron.clem@gmail.com>

From: Colin Didier <colin.didier@devialet.com>

Add driver for NXP's EPIT timer used in i.MX SoC.

Signed-off-by: Colin Didier <colin.didier@devialet.com>
Signed-off-by: Cl?ment Peron <clement.peron@devialet.com>
Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>
Tested-by: Vladimir Zapolskiy <vz@mleia.com>
---
 drivers/clocksource/Kconfig          |  11 ++
 drivers/clocksource/Makefile         |   1 +
 drivers/clocksource/timer-imx-epit.c | 265 +++++++++++++++++++++++++++
 3 files changed, 277 insertions(+)
 create mode 100644 drivers/clocksource/timer-imx-epit.c

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 8e8a09755d10..790478afd02c 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -576,6 +576,17 @@ config H8300_TPU
 	  This enables the clocksource for the H8300 platform with the
 	  H8S2678 cpu.
 
+config CLKSRC_IMX_EPIT
+	bool "Clocksource using i.MX EPIT"
+	depends on CLKDEV_LOOKUP && (ARCH_MXC || COMPILE_TEST)
+	select CLKSRC_MMIO
+	help
+	  This enables EPIT support available on some i.MX platforms.
+	  Normally you don't have a reason to do so as the EPIT has
+	  the same features and uses the same clocks as the GPT.
+	  Anyway, on some systems the GPT may be in use for other
+	  purposes.
+
 config CLKSRC_IMX_GPT
 	bool "Clocksource using i.MX GPT" if COMPILE_TEST
 	depends on ARM && CLKDEV_LOOKUP
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 00caf37e52f9..d9426f69ec69 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -69,6 +69,7 @@ obj-$(CONFIG_INTEGRATOR_AP_TIMER)	+= timer-integrator-ap.o
 obj-$(CONFIG_CLKSRC_VERSATILE)		+= versatile.o
 obj-$(CONFIG_CLKSRC_MIPS_GIC)		+= mips-gic-timer.o
 obj-$(CONFIG_CLKSRC_TANGO_XTAL)		+= tango_xtal.o
+obj-$(CONFIG_CLKSRC_IMX_EPIT)		+= timer-imx-epit.o
 obj-$(CONFIG_CLKSRC_IMX_GPT)		+= timer-imx-gpt.o
 obj-$(CONFIG_CLKSRC_IMX_TPM)		+= timer-imx-tpm.o
 obj-$(CONFIG_ASM9260_TIMER)		+= asm9260_timer.o
diff --git a/drivers/clocksource/timer-imx-epit.c b/drivers/clocksource/timer-imx-epit.c
new file mode 100644
index 000000000000..7f1c8e2e00aa
--- /dev/null
+++ b/drivers/clocksource/timer-imx-epit.c
@@ -0,0 +1,265 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * i.MX EPIT Timer
+ *
+ * Copyright (C) 2010 Sascha Hauer <s.hauer@pengutronix.de>
+ * Copyright (C) 2018 Colin Didier <colin.didier@devialet.com>
+ * Copyright (C) 2018 Cl?ment P?ron <clement.peron@devialet.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/clockchips.h>
+#include <linux/interrupt.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/sched_clock.h>
+#include <linux/slab.h>
+
+#define EPITCR				0x00
+#define EPITSR				0x04
+#define EPITLR				0x08
+#define EPITCMPR			0x0c
+#define EPITCNR				0x10
+
+#define EPITCR_EN			BIT(0)
+#define EPITCR_ENMOD			BIT(1)
+#define EPITCR_OCIEN			BIT(2)
+#define EPITCR_RLD			BIT(3)
+#define EPITCR_PRESC(x)			(((x) & 0xfff) << 4)
+#define EPITCR_SWR			BIT(16)
+#define EPITCR_IOVW			BIT(17)
+#define EPITCR_DBGEN			BIT(18)
+#define EPITCR_WAITEN			BIT(19)
+#define EPITCR_RES			BIT(20)
+#define EPITCR_STOPEN			BIT(21)
+#define EPITCR_OM_DISCON		(0 << 22)
+#define EPITCR_OM_TOGGLE		(1 << 22)
+#define EPITCR_OM_CLEAR			(2 << 22)
+#define EPITCR_OM_SET			(3 << 22)
+#define EPITCR_CLKSRC_OFF		(0 << 24)
+#define EPITCR_CLKSRC_PERIPHERAL	(1 << 24)
+#define EPITCR_CLKSRC_REF_HIGH		(2 << 24)
+#define EPITCR_CLKSRC_REF_LOW		(3 << 24)
+
+#define EPITSR_OCIF			BIT(0)
+
+struct epit_timer {
+	void __iomem *base;
+	int irq;
+	struct clk *clk;
+	struct clock_event_device ced;
+	struct irqaction act;
+};
+
+static void __iomem *sched_clock_reg;
+
+static inline struct epit_timer *to_epit_timer(struct clock_event_device *ced)
+{
+	return container_of(ced, struct epit_timer, ced);
+}
+
+static inline void epit_irq_disable(struct epit_timer *epittm)
+{
+	u32 val;
+
+	val = readl_relaxed(epittm->base + EPITCR);
+	writel_relaxed(val & ~EPITCR_OCIEN, epittm->base + EPITCR);
+}
+
+static inline void epit_irq_enable(struct epit_timer *epittm)
+{
+	u32 val;
+
+	val = readl_relaxed(epittm->base + EPITCR);
+	writel_relaxed(val | EPITCR_OCIEN, epittm->base + EPITCR);
+}
+
+static void epit_irq_acknowledge(struct epit_timer *epittm)
+{
+	writel_relaxed(EPITSR_OCIF, epittm->base + EPITSR);
+}
+
+static u64 notrace epit_read_sched_clock(void)
+{
+	return ~readl_relaxed(sched_clock_reg);
+}
+
+static int epit_set_next_event(unsigned long cycles,
+			       struct clock_event_device *ced)
+{
+	struct epit_timer *epittm = to_epit_timer(ced);
+	unsigned long tcmp;
+
+	tcmp = readl_relaxed(epittm->base + EPITCNR) - cycles;
+	writel_relaxed(tcmp, epittm->base + EPITCMPR);
+
+	return 0;
+}
+
+/* Left event sources disabled, no more interrupts appear */
+static int epit_shutdown(struct clock_event_device *ced)
+{
+	struct epit_timer *epittm = to_epit_timer(ced);
+	unsigned long flags;
+
+	/*
+	 * The timer interrupt generation is disabled at least
+	 * for enough time to call epit_set_next_event()
+	 */
+	local_irq_save(flags);
+
+	/* Disable interrupt in EPIT module */
+	epit_irq_disable(epittm);
+
+	/* Clear pending interrupt */
+	epit_irq_acknowledge(epittm);
+
+	local_irq_restore(flags);
+
+	return 0;
+}
+
+static int epit_set_oneshot(struct clock_event_device *ced)
+{
+	struct epit_timer *epittm = to_epit_timer(ced);
+	unsigned long flags;
+
+	/*
+	 * The timer interrupt generation is disabled at least
+	 * for enough time to call epit_set_next_event()
+	 */
+	local_irq_save(flags);
+
+	/* Disable interrupt in EPIT module */
+	epit_irq_disable(epittm);
+
+	/* Clear pending interrupt, only while switching mode */
+	if (!clockevent_state_oneshot(ced))
+		epit_irq_acknowledge(epittm);
+
+	/*
+	 * Do not put overhead of interrupt enable/disable into
+	 * epit_set_next_event(), the core has about 4 minutes
+	 * to call epit_set_next_event() or shutdown clock after
+	 * mode switching
+	 */
+	epit_irq_enable(epittm);
+	local_irq_restore(flags);
+
+	return 0;
+}
+
+static irqreturn_t epit_timer_interrupt(int irq, void *dev_id)
+{
+	struct clock_event_device *ced = dev_id;
+	struct epit_timer *epittm = to_epit_timer(ced);
+
+	epit_irq_acknowledge(epittm);
+
+	ced->event_handler(ced);
+
+	return IRQ_HANDLED;
+}
+
+static int __init epit_clocksource_init(struct epit_timer *epittm)
+{
+	unsigned int c = clk_get_rate(epittm->clk);
+
+	sched_clock_reg = epittm->base + EPITCNR;
+	sched_clock_register(epit_read_sched_clock, 32, c);
+
+	return clocksource_mmio_init(epittm->base + EPITCNR, "epit", c, 200, 32,
+				     clocksource_mmio_readl_down);
+}
+
+static int __init epit_clockevent_init(struct epit_timer *epittm)
+{
+	struct clock_event_device *ced = &epittm->ced;
+	struct irqaction *act = &epittm->act;
+
+	ced->name = "epit";
+	ced->features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_DYNIRQ;
+	ced->set_state_shutdown = epit_shutdown;
+	ced->tick_resume = epit_shutdown;
+	ced->set_state_oneshot = epit_set_oneshot;
+	ced->set_next_event = epit_set_next_event;
+	ced->rating = 200;
+	ced->cpumask = cpumask_of(0);
+	ced->irq = epittm->irq;
+	clockevents_config_and_register(ced, clk_get_rate(epittm->clk),
+					0xff, 0xfffffffe);
+
+	act->name = "i.MX EPIT Timer Tick",
+	act->flags = IRQF_TIMER | IRQF_IRQPOLL;
+	act->handler = epit_timer_interrupt;
+	act->dev_id = ced;
+
+	/* Make irqs happen */
+	return setup_irq(epittm->irq, act);
+}
+
+static int __init epit_timer_init(struct device_node *np)
+{
+	struct epit_timer *epittm;
+	int ret;
+
+	epittm = kzalloc(sizeof(*epittm), GFP_KERNEL);
+	if (!epittm)
+		return -ENOMEM;
+
+	epittm->base = of_iomap(np, 0);
+	if (!epittm->base) {
+		ret = -ENXIO;
+		goto out_kfree;
+	}
+
+	epittm->irq = irq_of_parse_and_map(np, 0);
+	if (!epittm->irq) {
+		ret = -EINVAL;
+		goto out_iounmap;
+	}
+
+	/* Get EPIT clock */
+	epittm->clk = of_clk_get(np, 0);
+	if (IS_ERR(epittm->clk)) {
+		pr_err("i.MX EPIT: unable to get clk\n");
+		ret = PTR_ERR(epittm->clk);
+		goto out_iounmap;
+	}
+
+	ret = clk_prepare_enable(epittm->clk);
+	if (ret) {
+		pr_err("i.MX EPIT: unable to prepare+enable clk\n");
+		goto out_iounmap;
+	}
+
+	/* Initialise to a known state (all timers off, and timing reset) */
+	writel_relaxed(0x0, epittm->base + EPITCR);
+	writel_relaxed(0xffffffff, epittm->base + EPITLR);
+	writel_relaxed(EPITCR_EN | EPITCR_CLKSRC_REF_HIGH | EPITCR_WAITEN,
+		       epittm->base + EPITCR);
+
+	ret = epit_clocksource_init(epittm);
+	if (ret) {
+		pr_err("i.MX EPIT: failed to init clocksource\n");
+		goto out_clk_disable;
+	}
+
+	ret = epit_clockevent_init(epittm);
+	if (ret) {
+		pr_err("i.MX EPIT: failed to init clockevent\n");
+		goto out_clk_disable;
+	}
+
+	return 0;
+
+out_clk_disable:
+	clk_disable_unprepare(epittm->clk);
+out_iounmap:
+	iounmap(epittm->base);
+out_kfree:
+	kfree(epittm);
+
+	return ret;
+}
+TIMER_OF_DECLARE(epit_timer, "fsl,imx31-epit", epit_timer_init);
-- 
2.17.1

^ permalink raw reply related

* [PATCH v7 5/5] ARM: dts: imx: add missing compatible and clock properties for EPIT
From: Clément Péron @ 2018-06-11 15:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180611155001.3506-1-peron.clem@gmail.com>

From: Colin Didier <colin.didier@devialet.com>

Add missing compatible and clock properties for EPIT node.

Signed-off-by: Colin Didier <colin.didier@devialet.com>
Signed-off-by: Cl?ment Peron <clement.peron@devialet.com>
Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>
---
 arch/arm/boot/dts/imx25.dtsi   |  8 ++++++--
 arch/arm/boot/dts/imx6qdl.dtsi | 10 ++++++++--
 arch/arm/boot/dts/imx6sl.dtsi  | 10 ++++++++--
 arch/arm/boot/dts/imx6sx.dtsi  | 10 ++++++++--
 arch/arm/boot/dts/imx6ul.dtsi  | 10 ++++++++--
 5 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi
index cf70df20b19c..15fd4308dad8 100644
--- a/arch/arm/boot/dts/imx25.dtsi
+++ b/arch/arm/boot/dts/imx25.dtsi
@@ -396,15 +396,19 @@
 			};
 
 			epit1: timer at 53f94000 {
-				compatible = "fsl,imx25-epit";
+				compatible = "fsl,imx25-epit", "fsl,imx31-epit";
 				reg = <0x53f94000 0x4000>;
 				interrupts = <28>;
+				clocks = <&clks 83>;
+				status = "disabled";
 			};
 
 			epit2: timer at 53f98000 {
-				compatible = "fsl,imx25-epit";
+				compatible = "fsl,imx25-epit", "fsl,imx31-epit";
 				reg = <0x53f98000 0x4000>;
 				interrupts = <27>;
+				clocks = <&clks 84>;
+				status = "disabled";
 			};
 
 			gpio4: gpio at 53f9c000 {
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index c003e62bf290..65c4ee07454c 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -843,14 +843,20 @@
 				};
 			};
 
-			epit1: epit at 20d0000 { /* EPIT1 */
+			epit1: timer at 20d0000 {
+				compatible = "fsl,imx6qdl-epit", "fsl,imx31-epit";
 				reg = <0x020d0000 0x4000>;
 				interrupts = <0 56 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clks IMX6QDL_CLK_EPIT1>;
+				status = "disabled";
 			};
 
-			epit2: epit at 20d4000 { /* EPIT2 */
+			epit2: timer at 20d4000 {
+				compatible = "fsl,imx6qdl-epit", "fsl,imx31-epit";
 				reg = <0x020d4000 0x4000>;
 				interrupts = <0 57 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clks IMX6QDL_CLK_EPIT2>;
+				status = "disabled";
 			};
 
 			src: src at 20d8000 {
diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
index ab6a7e2e7e8f..d63f8ebbc8a1 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -671,14 +671,20 @@
 				};
 			};
 
-			epit1: epit at 20d0000 {
+			epit1: timer at 20d0000 {
+				compatible = "fsl,imx6sl-epit", "fsl,imx31-epit";
 				reg = <0x020d0000 0x4000>;
 				interrupts = <0 56 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clks IMX6SL_CLK_EPIT1>;
+				status = "disabled";
 			};
 
-			epit2: epit at 20d4000 {
+			epit2: timer at 20d4000 {
+				compatible = "fsl,imx6sl-epit", "fsl,imx31-epit";
 				reg = <0x020d4000 0x4000>;
 				interrupts = <0 57 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clks IMX6SL_CLK_EPIT2>;
+				status = "disabled";
 			};
 
 			src: src at 20d8000 {
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index 49c7205b8db8..2b30559d3270 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -736,14 +736,20 @@
 				};
 			};
 
-			epit1: epit at 20d0000 {
+			epit1: timer at 20d0000 {
+				compatible = "fsl,imx6sx-epit", "fsl,imx31-epit";
 				reg = <0x020d0000 0x4000>;
 				interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clks IMX6SX_CLK_EPIT1>;
+				status = "disabled";
 			};
 
-			epit2: epit at 20d4000 {
+			epit2: timer at 20d4000 {
+				compatible = "fsl,imx6sx-epit", "fsl,imx31-epit";
 				reg = <0x020d4000 0x4000>;
 				interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clks IMX6SX_CLK_EPIT2>;
+				status = "disabled";
 			};
 
 			src: src at 20d8000 {
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index 1241972b16ba..d5f765da1ee2 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -658,14 +658,20 @@
 				};
 			};
 
-			epit1: epit at 20d0000 {
+			epit1: timer at 20d0000 {
+				compatible = "fsl,imx6ul-epit", "fsl,imx31-epit";
 				reg = <0x020d0000 0x4000>;
 				interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clks IMX6UL_CLK_EPIT1>;
+				status = "disabled";
 			};
 
-			epit2: epit at 20d4000 {
+			epit2: timer at 20d4000 {
+				compatible = "fsl,imx6ul-epit", "fsl,imx31-epit";
 				reg = <0x020d4000 0x4000>;
 				interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clks IMX6UL_CLK_EPIT2>;
+				status = "disabled";
 			};
 
 			src: src at 20d8000 {
-- 
2.17.1

^ permalink raw reply related

* [PATCH v5 4/4] kernel hacking: new config CC_OPTIMIZE_FOR_DEBUGGING to apply GCC -Og optimization
From: Steven Rostedt @ 2018-06-11 16:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <201806101929.E2oiistg%fengguang.wu@intel.com>

On Sun, 10 Jun 2018 23:49:55 +0800
kbuild test robot <lkp@intel.com> wrote:

> Hi Changbin,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on linus/master]
> [also build test WARNING on v4.17 next-20180608]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/changbin-du-intel-com/kernel-hacking-GCC-optimization-for-better-debug-experience-Og/20180606-001415
> config: i386-randconfig-x076-06101602 (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386 
> 
> Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
> http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
> 
> All warnings (new ones prefixed by >>):
> 
>    drivers//usb/typec/fusb302/fusb302.c: In function 'fusb302_handle_togdone_src':
> >> drivers//usb/typec/fusb302/fusb302.c:1413:10: warning: 'ra_comp' may be used uninitialized in this function [-Wmaybe-uninitialized]  
>      else if (ra_comp)
>              ^

This is a false warning. I'm surprised gcc couldn't catch it. Although
that code looks like it could have been  done a bit nicer.


> --
>    drivers/infiniband/ulp/ipoib/ipoib_main.c: In function 'ipoib_get_netdev':
> >> drivers/infiniband/ulp/ipoib/ipoib_main.c:2021:30: warning: 'dev' may be used uninitialized in this function [-Wmaybe-uninitialized]  
>      if (!hca->alloc_rdma_netdev || PTR_ERR(dev) == -EOPNOTSUPP)
> --

Strange, this is also false, with the same construct.

	if (a) {
		b = init;
	}
	if (!a) {
		use b;

It warns that b may be unused. I'm guessing the extra option we add in
gcc by the patch causes gcc to break in this regard.



>    kernel//cgroup/cgroup-v1.c: In function 'cgroup1_mount':
> >> kernel//cgroup/cgroup-v1.c:1268:3: warning: 'root' may be used uninitialized in this function [-Wmaybe-uninitialized]  
>       percpu_ref_reinit(&root->cgrp.self.refcnt);
>       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> --

Slightly different construct, but similar:

	ret = func();
	if (ret)
		goto out_unlock;

	root = init;

 out_unlock:

	if (ret)
		return;

	use root;



>    kernel//trace/bpf_trace.c: In function 'bpf_trace_printk':
> >> kernel//trace/bpf_trace.c:226:20: warning: 'unsafe_addr' may be used uninitialized in this function [-Wmaybe-uninitialized]  
>               (void *) (long) unsafe_addr,
>                        ^~~~~~~~~~~~~~~~~~

Again similar:

	if (fmt_cnt >= 3)
		return;

	switch (fmt_cnt) {
	case 1:
		unsafe_addr = init;
		break;
	case 2:
		unsafe_addr = init2;
		break;
	case 3:
		unsafe_addr = init3;
		break;
	}

	use init;


>    kernel//trace/bpf_trace.c:170:6: note: 'unsafe_addr' was declared here
>      u64 unsafe_addr;
>          ^~~~~~~~~~~
> --
>    net//6lowpan/iphc.c: In function 'lowpan_header_decompress':
>    net//6lowpan/iphc.c:617:12: warning: 'iphc1' may be used uninitialized in this function [-Wmaybe-uninitialized]
>      u8 iphc0, iphc1, cid = 0;
>                ^~~~~
> >> net//6lowpan/iphc.c:617:5: warning: 'iphc0' may be used uninitialized in this function [-Wmaybe-uninitialized]  
>      u8 iphc0, iphc1, cid = 0;
>         ^~~~~

Similar but crazier:

	if (lowpan_fetch_skb(&iphc0) ||
	    lowpan_fetch_skb(&iphc1))
		return;

	use iphc0 and ipch1;

where lowpan_fetch_skb() is:

	if (test())
		return true;

	init data (iphc0 or iphc1);
	return false;


> --
>    net//netfilter/nf_tables_api.c: In function 'nf_tables_dump_set':
> >> net//netfilter/nf_tables_api.c:3625:2: warning: 'set' may be used uninitialized in this function [-Wmaybe-uninitialized]  
>      set->ops->walk(&dump_ctx->ctx, set, &args.iter);
>      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I don't have the same kernel, as this doesn't match. But I'm sure it's
a false positive like the others.


> --
>    drivers/media/dvb-frontends/mn88472.c: In function 'mn88472_set_frontend':
> >> drivers/media/dvb-frontends/mn88472.c:339:27: warning: 'bandwidth_vals_ptr' may be used uninitialized in this function [-Wmaybe-uninitialized]  
>             bandwidth_vals_ptr[i]);
>                               ^
> >> drivers/media/dvb-frontends/mn88472.c:320:8: warning: 'bandwidth_val' may be used uninitialized in this function [-Wmaybe-uninitialized]  
>      ret = regmap_write(dev->regmap[2], 0x04, bandwidth_val);
>            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This one may not be a false positive. It really looks like there's a
path to that being used uninitialized. But I haven't torn that function
apart enough to really tell, but I don't fault gcc for not warning
about it. But I like to know if gcc doesn't warn without this patch?


> --
>    drivers/media/dvb-frontends/mn88473.c: In function 'mn88473_set_frontend':
> >> drivers/media/dvb-frontends/mn88473.c:162:7: warning: 'conf_val_ptr' may be used uninitialized in this function [-Wmaybe-uninitialized]  
>       ret = regmap_bulk_write(dev->regmap[1], 0x10, conf_val_ptr, 6);
>       ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Same as the one before it. Need to see if this isn't really a real
issue.

> --
>    net//netfilter/ipvs/ip_vs_sync.c: In function 'ip_vs_sync_conn':
> >> net//netfilter/ipvs/ip_vs_sync.c:731:13: warning: 'm' may be used uninitialized in this function [-Wmaybe-uninitialized]  
>      m->nr_conns++;
>      ~~~~~~~~~~~^~

gcc is really stupid on this one.

	if (buff)
		init m;
	if (!buff)
		init m;

	use m;

Really?

> --
>    drivers//hwspinlock/hwspinlock_core.c: In function 'of_hwspin_lock_get_id':
> >> drivers//hwspinlock/hwspinlock_core.c:339:19: warning: 'id' may be used uninitialized in this function [-Wmaybe-uninitialized]  
>      return ret ? ret : id;
>             ~~~~~~~~~~^~~~


Again, we jump here without initializing 'id' when ret is set.


> --
>    drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c: In function 'mlxsw_sp_nexthop_group_update':
> >> drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c:3078:7: warning: 'err' may be used uninitialized in this function [-Wmaybe-uninitialized]  
>        if (err)
>           ^
> 
> vim +/ra_comp +1413 drivers//usb/typec/fusb302/fusb302.c
> 


Another switch statement false positive:

	nh->type can only be set to two different values, and then we
	have:

	switch (nh->type) {
	case value1:
		err = func();
		break;
	case value2:
		err = func2();
		break;
	}
	if (err)


Of all the warnings, only one looks like it could be a possible issue.
Thus, this patch causes gcc to fail more on it analysis. The one
possible issue should have been caught by gcc without this patch, so
I'm skeptical that it is indeed an issue, but it's complex and I am
impressed if gcc really did figure it out.

-- Steve

^ permalink raw reply

* [PATCH v2 03/40] iommu/sva: Manage process address spaces
From: Kenneth Lee @ 2018-06-11 16:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180526022445.GA6069@kllp05>

On Sat, May 26, 2018 at 10:24:45AM +0800, Kenneth Lee wrote:
> Date: Sat, 26 May 2018 10:24:45 +0800
> From: Kenneth Lee <Kenneth-Lee-2012@foxmail.com>
> To: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>, Jean-Philippe Brucker
>  <jean-philippe.brucker@arm.com>, "xieyisheng1 at huawei.com"
>  <xieyisheng1@huawei.com>, "kvm at vger.kernel.org" <kvm@vger.kernel.org>,
>  "linux-pci at vger.kernel.org" <linux-pci@vger.kernel.org>,
>  "xuzaibo at huawei.com" <xuzaibo@huawei.com>, Will Deacon
>  <Will.Deacon@arm.com>, "okaya at codeaurora.org" <okaya@codeaurora.org>,
>  "linux-mm at kvack.org" <linux-mm@kvack.org>, "yi.l.liu at intel.com"
>  <yi.l.liu@intel.com>, "ashok.raj at intel.com" <ashok.raj@intel.com>,
>  "tn at semihalf.com" <tn@semihalf.com>, "joro at 8bytes.org" <joro@8bytes.org>,
>  "robdclark at gmail.com" <robdclark@gmail.com>, "bharatku at xilinx.com"
>  <bharatku@xilinx.com>, "linux-acpi at vger.kernel.org"
>  <linux-acpi@vger.kernel.org>, "liudongdong3 at huawei.com"
>  <liudongdong3@huawei.com>, "rfranz at cavium.com" <rfranz@cavium.com>,
>  "devicetree at vger.kernel.org" <devicetree@vger.kernel.org>,
>  "kevin.tian at intel.com" <kevin.tian@intel.com>, Jacob Pan
>  <jacob.jun.pan@linux.intel.com>, "alex.williamson at redhat.com"
>  <alex.williamson@redhat.com>, "rgummal at xilinx.com" <rgummal@xilinx.com>,
>  "thunder.leizhen at huawei.com" <thunder.leizhen@huawei.com>,
>  "linux-arm-kernel at lists.infradead.org"
>  <linux-arm-kernel@lists.infradead.org>, "shunyong.yang at hxt-semitech.com"
>  <shunyong.yang@hxt-semitech.com>, "dwmw2 at infradead.org"
>  <dwmw2@infradead.org>, "liubo95 at huawei.com" <liubo95@huawei.com>,
>  "jcrouse at codeaurora.org" <jcrouse@codeaurora.org>,
>  "iommu at lists.linux-foundation.org" <iommu@lists.linux-foundation.org>,
>  Robin Murphy <Robin.Murphy@arm.com>, "christian.koenig at amd.com"
>  <christian.koenig@amd.com>, "nwatters at codeaurora.org"
>  <nwatters@codeaurora.org>, "baolu.lu at linux.intel.com"
>  <baolu.lu@linux.intel.com>, liguozhu at hisilicon.com
> Subject: Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces
> Message-ID: <20180526022445.GA6069@kllp05>
> 
> On Fri, May 25, 2018 at 09:39:59AM +0100, Jonathan Cameron wrote:
> > Date: Fri, 25 May 2018 09:39:59 +0100
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > To: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > CC: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>,
> >  "xieyisheng1 at huawei.com" <xieyisheng1@huawei.com>, "kvm at vger.kernel.org"
> >  <kvm@vger.kernel.org>, "linux-pci at vger.kernel.org"
> >  <linux-pci@vger.kernel.org>, "xuzaibo at huawei.com" <xuzaibo@huawei.com>,
> >  Will Deacon <Will.Deacon@arm.com>, "okaya at codeaurora.org"
> >  <okaya@codeaurora.org>, "linux-mm at kvack.org" <linux-mm@kvack.org>,
> >  "yi.l.liu at intel.com" <yi.l.liu@intel.com>, "ashok.raj at intel.com"
> >  <ashok.raj@intel.com>, "tn at semihalf.com" <tn@semihalf.com>,
> >  "joro at 8bytes.org" <joro@8bytes.org>, "robdclark at gmail.com"
> >  <robdclark@gmail.com>, "bharatku at xilinx.com" <bharatku@xilinx.com>,
> >  "linux-acpi at vger.kernel.org" <linux-acpi@vger.kernel.org>,
> >  "liudongdong3 at huawei.com" <liudongdong3@huawei.com>, "rfranz at cavium.com"
> >  <rfranz@cavium.com>, "devicetree at vger.kernel.org"
> >  <devicetree@vger.kernel.org>, "kevin.tian at intel.com"
> >  <kevin.tian@intel.com>, Jacob Pan <jacob.jun.pan@linux.intel.com>,
> >  "alex.williamson at redhat.com" <alex.williamson@redhat.com>,
> >  "rgummal at xilinx.com" <rgummal@xilinx.com>, "thunder.leizhen at huawei.com"
> >  <thunder.leizhen@huawei.com>, "linux-arm-kernel at lists.infradead.org"
> >  <linux-arm-kernel@lists.infradead.org>, "shunyong.yang at hxt-semitech.com"
> >  <shunyong.yang@hxt-semitech.com>, "dwmw2 at infradead.org"
> >  <dwmw2@infradead.org>, "liubo95 at huawei.com" <liubo95@huawei.com>,
> >  "jcrouse at codeaurora.org" <jcrouse@codeaurora.org>,
> >  "iommu at lists.linux-foundation.org" <iommu@lists.linux-foundation.org>,
> >  Robin Murphy <Robin.Murphy@arm.com>, "christian.koenig at amd.com"
> >  <christian.koenig@amd.com>, "nwatters at codeaurora.org"
> >  <nwatters@codeaurora.org>, "baolu.lu at linux.intel.com"
> >  <baolu.lu@linux.intel.com>, liguozhu at hisilicon.com,
> >  kenneth-lee-2012 at foxmail.com
> > Subject: Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces
> > Message-ID: <20180525093959.000040a7@huawei.com>
> > 
> > +CC Kenneth Lee
> > 
> > On Fri, 25 May 2018 09:33:11 +0300
> > Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote:
> > 
> > > On Thu, May 24, 2018 at 04:04:39PM +0100, Jean-Philippe Brucker wrote:
> > > > On 24/05/18 12:50, Ilias Apalodimas wrote:  
> > > > >> Interesting, I hadn't thought about this use-case before. At first I
> > > > >> thought you were talking about mdev devices assigned to VMs, but I think
> > > > >> you're referring to mdevs assigned to userspace drivers instead? Out of
> > > > >> curiosity, is it only theoretical or does someone actually need this?  
> > > > > 
> > > > > There has been some non upstreamed efforts to have mdev and produce userspace
> > > > > drivers. Huawei is using it on what they call "wrapdrive" for crypto devices and
> > > > > we did a proof of concept for ethernet interfaces. At the time we choose not to
> > > > > involve the IOMMU for the reason you mentioned, but having it there would be
> > > > > good.  
> > > > 
> > > > I'm guessing there were good reasons to do it that way but I wonder, is
> > > > it not simpler to just have the kernel driver create a /dev/foo, with a
> > > > standard ioctl/mmap/poll interface? Here VFIO adds a layer of
> > > > indirection, and since the mediating driver has to implement these
> > > > operations already, what is gained?  
> > > The best reason i can come up with is "common code". You already have one API
> > > doing that for you so we replicate it in a /dev file?
> > > The mdev approach still needs extentions to support what we tried to do (i.e
> > > mdev bus might need yo have access on iommu_ops), but as far as i undestand it's
> > > a possible case.
> 
> Hi, Jean, Please allow me to share my understanding here:
> https://zhuanlan.zhihu.com/p/35489035
> 
> The reason we do not use the /dev/foo scheme is that the devices to be
> shared are programmable accelerators. We cannot fix up the kernel driver for them.
> > > > 
> > > > Thanks,
> > > > Jean  
> > 
> > 
> 
> -- 
> 			-Kenneth Lee (Hisilicon)

I just found this mail was missed in the mailing list. I tried it once
again.

-- 
			-Kenneth Lee (Hisilicon)

^ permalink raw reply

* [PATCH] Documentation: Fix reference to stm.txt
From: Mathieu Poirier @ 2018-06-11 16:15 UTC (permalink / raw)
  To: linux-arm-kernel

Commit "1606f8d8e75b trace doc: convert trace/stm.txt to rst format"
changed stm.txt to stm.rst but references to it in other files have not
been modified, something that is corrected in this patch.

Reported-by: Joe Perches <joe@perches.com>
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 Documentation/trace/coresight.txt | 2 +-
 Documentation/trace/intel_th.rst  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/trace/coresight.txt b/Documentation/trace/coresight.txt
index 1d74ad0202b6..efbc832146e7 100644
--- a/Documentation/trace/coresight.txt
+++ b/Documentation/trace/coresight.txt
@@ -426,5 +426,5 @@ root at genericarmv8:~#
 Details on how to use the generic STM API can be found here [2].
 
 [1]. Documentation/ABI/testing/sysfs-bus-coresight-devices-stm
-[2]. Documentation/trace/stm.txt
+[2]. Documentation/trace/stm.rst
 [3]. https://github.com/Linaro/perf-opencsd
diff --git a/Documentation/trace/intel_th.rst b/Documentation/trace/intel_th.rst
index 990f13265178..fdb34114ee20 100644
--- a/Documentation/trace/intel_th.rst
+++ b/Documentation/trace/intel_th.rst
@@ -38,7 +38,7 @@ description is at Documentation/ABI/testing/sysfs-bus-intel_th-devices-gth.
 
 STH registers an stm class device, through which it provides interface
 to userspace and kernelspace software trace sources. See
-Documentation/trace/stm.txt for more information on that.
+Documentation/trace/stm.rst for more information on that.
 
 MSU can be configured to collect trace data into a system memory
 buffer, which can later on be read from its device nodes via read() or
@@ -89,7 +89,7 @@ Quick example
 
 	$ echo 1 > /sys/bus/intel_th/devices/0-msc0/active
 
-# .. send data to master 33, see stm.txt for more details ..
+# .. send data to master 33, see stm.rst for more details ..
 # .. wait for traces to pile up ..
 # .. and stop the trace::
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH] arm/twd: avoid waking deeply sleeping CPUs for rate change notifier
From: Lucas Stach @ 2018-06-11 16:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180417104627.29643-1-l.stach@pengutronix.de>

Hi all,

Am Dienstag, den 17.04.2018, 12:46 +0200 schrieb Lucas Stach:
> The current clock notifier sends an IPI to all CPUs, even if they are in
> deep sleep state with the local timer disabled and switched to tick
> broadcast. This needlessly cuts the CPU sleep times, as nothing is gained
> from updating a disabled TWDs rate.
> 
> Keep track of the enabled TWDs and only send an IPI to those CPUs with an
> active local timer. As disabled TWDs may now miss a CPU frequency update
> we need to make sure to refresh the rate on re-enabling of the timer.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

I would appreciate some feedback to this patch.
FWIW, I've been running some systems with this patch applied for quite
some time now with no issues spotted so far.

Regards,
Lucas

> ---
> ?arch/arm/kernel/smp_twd.c | 31 +++++++++++++++++++++++++++++--
> ?1 file changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
> index b30eafeef096..a3be30d30cc2 100644
> --- a/arch/arm/kernel/smp_twd.c
> +++ b/arch/arm/kernel/smp_twd.c
> @@ -32,6 +32,8 @@ static struct clk *twd_clk;
> ?static unsigned long twd_timer_rate;
> ?static DEFINE_PER_CPU(bool, percpu_setup_called);
> ?
> +static cpumask_var_t active_twd_mask;
> +
> ?static struct clock_event_device __percpu *twd_evt;
> ?static unsigned int twd_features =
> > ?		CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
> @@ -39,12 +41,24 @@ static int twd_ppi;
> ?
> ?static int twd_shutdown(struct clock_event_device *clk)
> ?{
> > +	cpumask_clear_cpu(smp_processor_id(), active_twd_mask);
> +
> > ?	writel_relaxed(0, twd_base + TWD_TIMER_CONTROL);
> > ?	return 0;
> ?}
> ?
> ?static int twd_set_oneshot(struct clock_event_device *clk)
> ?{
> > +	cpumask_set_cpu(smp_processor_id(), active_twd_mask);
> +
> > +	/*
> > +	?* When going from shutdown to oneshot we might have missed some
> > +	?* frequency updates if the CPU was sleeping. Make sure to update
> > +	?* the timer frequency with the current rate.
> > +	?*/
> > +	if (clockevent_state_shutdown(clk))
> > +		clockevents_update_freq(clk, twd_timer_rate);
> +
> > ?	/* period set, and timer enabled in 'next_event' hook */
> > ?	writel_relaxed(TWD_TIMER_CONTROL_IT_ENABLE | TWD_TIMER_CONTROL_ONESHOT,
> > ?		???????twd_base + TWD_TIMER_CONTROL);
> @@ -57,6 +71,16 @@ static int twd_set_periodic(struct clock_event_device *clk)
> > ?			?????TWD_TIMER_CONTROL_IT_ENABLE |
> > ?			?????TWD_TIMER_CONTROL_PERIODIC;
> ?
> > +	cpumask_set_cpu(smp_processor_id(), active_twd_mask);
> +
> > +	/*
> > +	?* When going from shutdown to periodic we might have missed some
> > +	?* frequency updates if the CPU was sleeping. Make sure to update
> > +	?* the timer frequency with the current rate.
> > +	?*/
> > +	if (clockevent_state_shutdown(clk))
> > +		clockevents_update_freq(clk, twd_timer_rate);
> +
> > ?	writel_relaxed(DIV_ROUND_CLOSEST(twd_timer_rate, HZ),
> > ?		???????twd_base + TWD_TIMER_LOAD);
> > ?	writel_relaxed(ctrl, twd_base + TWD_TIMER_CONTROL);
> @@ -124,8 +148,8 @@ static int twd_rate_change(struct notifier_block *nb,
> > ?	?* changing cpu.
> > ?	?*/
> > ?	if (flags == POST_RATE_CHANGE)
> > -		on_each_cpu(twd_update_frequency,
> > -				??(void *)&cnd->new_rate, 1);
> > +		on_each_cpu_mask(active_twd_mask, twd_update_frequency,
> > +				?(void *)&cnd->new_rate, 1);
> ?
> > ?	return NOTIFY_OK;
> ?}
> @@ -326,6 +350,9 @@ static int __init twd_local_timer_common_register(struct device_node *np)
> ?{
> > ?	int err;
> ?
> > +	if (!zalloc_cpumask_var(&active_twd_mask, GFP_KERNEL))
> > +		return -ENOMEM;
> +
> > ?	twd_evt = alloc_percpu(struct clock_event_device);
> > ?	if (!twd_evt) {
> > ?		err = -ENOMEM;

^ permalink raw reply

* [PATCH v2 5/5] arm64: perf: Add support for chaining event counters
From: Suzuki K Poulose @ 2018-06-11 16:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180611142431.4svc26y27scgzfcg@lakrids.cambridge.arm.com>

On 11/06/18 15:24, Mark Rutland wrote:
> On Mon, Jun 11, 2018 at 02:54:16PM +0100, Suzuki K Poulose wrote:
>> On 08/06/18 15:46, Suzuki K Poulose wrote:
>>> On 06/06/2018 07:01 PM, Mark Rutland wrote:
>>>> On Tue, May 29, 2018 at 11:55:56AM +0100, Suzuki K Poulose wrote:
>>
>>>>> -??????? value |= 0xffffffff00000000ULL;
>>>>> +??????? if (!armv8pmu_event_is_64bit(event))
>>>>> +??????????? value |= 0xffffffff00000000ULL;
>>>>>  ????????? write_sysreg(value, pmccntr_el0);
>>>>> -??? } else if (armv8pmu_select_counter(idx) == idx)
>>>>> -??????? write_sysreg(value, pmxevcntr_el0);
>>>>> +??? } else
>>>>> +??????? armv8pmu_write_hw_counter(event, value);
>>>>>  ? }
>>>>
>>>>> +static inline void armv8pmu_write_event_type(struct perf_event *event)
>>>>> +{
>>>>> +??? struct hw_perf_event *hwc = &event->hw;
>>>>> +??? int idx = hwc->idx;
>>>>> +
>>>>> +??? /*
>>>>> +???? * For chained events, write the the low counter event type
>>>>> +???? * followed by the high counter. The high counter is programmed
>>>>> +???? * with CHAIN event code with filters set to count at all ELs.
>>>>> +???? */
>>>>> +??? if (armv8pmu_event_is_chained(event)) {
>>>>> +??????? u32 chain_evt = ARMV8_PMUV3_PERFCTR_CHAIN |
>>>>> +??????????????? ARMV8_PMU_INCLUDE_EL2;
>>>>> +
>>>>> +??????? armv8pmu_write_evtype(idx - 1, hwc->config_base);
>>>>> +??????? isb();
>>>>> +??????? armv8pmu_write_evtype(idx, chain_evt);
>>>>
>>>> The ISB isn't necessary here, AFAICT. We only do this while the PMU is
>>>> disabled; no?
>>>
>>> You're right. I was just following the ARM ARM.
>>
>> Taking another look, it is not clear about the semantics of "pmu->enable()"
>> and pmu->disable() callbacks.
> 
> I was talking about pmu::{pmu_disable,pmu_enable}(), so I'm not sure I
> follow how arm_pmu::{enable,disable}() are relevant here.> 
> The arm_pmu::{enable,disable}() callbacks enable or disable individual
> counters. For example, leaving unused counters disabled may save power,
> even if the PMU as a whole is enabled.

Ah, I mistook cpu_pmu->enable/disable for the core pmu ops. My bad.
Sorry about the noise.

Suzuki

^ permalink raw reply

* [PATCH v2 03/40] iommu/sva: Manage process address spaces
From: Kenneth Lee @ 2018-06-11 16:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180525093959.000040a7@huawei.com>

On Fri, May 25, 2018 at 09:39:59AM +0100, Jonathan Cameron wrote:
> Date: Fri, 25 May 2018 09:39:59 +0100
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> To: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> CC: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>,
>  "xieyisheng1 at huawei.com" <xieyisheng1@huawei.com>, "kvm at vger.kernel.org"
>  <kvm@vger.kernel.org>, "linux-pci at vger.kernel.org"
>  <linux-pci@vger.kernel.org>, "xuzaibo at huawei.com" <xuzaibo@huawei.com>,
>  Will Deacon <Will.Deacon@arm.com>, "okaya at codeaurora.org"
>  <okaya@codeaurora.org>, "linux-mm at kvack.org" <linux-mm@kvack.org>,
>  "yi.l.liu at intel.com" <yi.l.liu@intel.com>, "ashok.raj at intel.com"
>  <ashok.raj@intel.com>, "tn at semihalf.com" <tn@semihalf.com>,
>  "joro at 8bytes.org" <joro@8bytes.org>, "robdclark at gmail.com"
>  <robdclark@gmail.com>, "bharatku at xilinx.com" <bharatku@xilinx.com>,
>  "linux-acpi at vger.kernel.org" <linux-acpi@vger.kernel.org>,
>  "liudongdong3 at huawei.com" <liudongdong3@huawei.com>, "rfranz at cavium.com"
>  <rfranz@cavium.com>, "devicetree at vger.kernel.org"
>  <devicetree@vger.kernel.org>, "kevin.tian at intel.com"
>  <kevin.tian@intel.com>, Jacob Pan <jacob.jun.pan@linux.intel.com>,
>  "alex.williamson at redhat.com" <alex.williamson@redhat.com>,
>  "rgummal at xilinx.com" <rgummal@xilinx.com>, "thunder.leizhen at huawei.com"
>  <thunder.leizhen@huawei.com>, "linux-arm-kernel at lists.infradead.org"
>  <linux-arm-kernel@lists.infradead.org>, "shunyong.yang at hxt-semitech.com"
>  <shunyong.yang@hxt-semitech.com>, "dwmw2 at infradead.org"
>  <dwmw2@infradead.org>, "liubo95 at huawei.com" <liubo95@huawei.com>,
>  "jcrouse at codeaurora.org" <jcrouse@codeaurora.org>,
>  "iommu at lists.linux-foundation.org" <iommu@lists.linux-foundation.org>,
>  Robin Murphy <Robin.Murphy@arm.com>, "christian.koenig at amd.com"
>  <christian.koenig@amd.com>, "nwatters at codeaurora.org"
>  <nwatters@codeaurora.org>, "baolu.lu at linux.intel.com"
>  <baolu.lu@linux.intel.com>, liguozhu at hisilicon.com,
>  kenneth-lee-2012 at foxmail.com
> Subject: Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces
> Message-ID: <20180525093959.000040a7@huawei.com>
> Organization: Huawei
> X-Mailer: Claws Mail 3.15.0 (GTK+ 2.24.31; x86_64-w64-mingw32)
> 
> +CC Kenneth Lee
> 
> On Fri, 25 May 2018 09:33:11 +0300
> Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote:
> 
> > On Thu, May 24, 2018 at 04:04:39PM +0100, Jean-Philippe Brucker wrote:
> > > On 24/05/18 12:50, Ilias Apalodimas wrote:  
> > > >> Interesting, I hadn't thought about this use-case before. At first I
> > > >> thought you were talking about mdev devices assigned to VMs, but I think
> > > >> you're referring to mdevs assigned to userspace drivers instead? Out of
> > > >> curiosity, is it only theoretical or does someone actually need this?  
> > > > 
> > > > There has been some non upstreamed efforts to have mdev and produce userspace
> > > > drivers. Huawei is using it on what they call "wrapdrive" for crypto devices and
> > > > we did a proof of concept for ethernet interfaces. At the time we choose not to
> > > > involve the IOMMU for the reason you mentioned, but having it there would be
> > > > good.  
> > > 
> > > I'm guessing there were good reasons to do it that way but I wonder, is
> > > it not simpler to just have the kernel driver create a /dev/foo, with a
> > > standard ioctl/mmap/poll interface? Here VFIO adds a layer of
> > > indirection, and since the mediating driver has to implement these
> > > operations already, what is gained?  
> > The best reason i can come up with is "common code". You already have one API
> > doing that for you so we replicate it in a /dev file?
> > The mdev approach still needs extentions to support what we tried to do (i.e
> > mdev bus might need yo have access on iommu_ops), but as far as i undestand it's
> > a possible case.

Hi, Jean, Please allow me to share my understanding here:
https://zhuanlan.zhihu.com/p/35489035

The reason we do not use the /dev/foo scheme is that the devices to be
shared are programmable accelerators. We cannot fix up the kernel driver for
them.

> > > 
> > > Thanks,
> > > Jean  
> 
> 

(p.s. I sent this mail on May 26 from my public email count. But it
seems the email server is blocked. I resent it from my company count until my
colleague told me just now. Sorry for inconvenience)

-- 
			-Kenneth(Hisilicon)

^ permalink raw reply

* [PATCH 08/20] coresight: dts: Cleanup device tree graph bindings
From: Mathieu Poirier @ 2018-06-11 16:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <f616c825-b459-397d-3e61-70c1f1790f4f@arm.com>

On 11 June 2018 at 03:22, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote:
> On 08/06/18 22:22, Mathieu Poirier wrote:
>>
>> On Tue, Jun 05, 2018 at 10:43:19PM +0100, Suzuki K Poulose wrote:
>>>
>>> The coresight drivers relied on default bindings for graph
>>> in DT, while reusing the "reg" field of the "ports" to indicate
>>> the actual hardware port number for the connections. However,
>>> with the rules getting stricter w.r.t to the address mismatch
>>> with the label, it is no longer possible to use the port address
>>> field for the hardware port number. Hence, we add an explicit
>>> property to denote the hardware port number, "coresight,hwid"
>>> which must be specified for each "endpoint".
>>>
>>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>>> Cc: Sudeep Holla <sudeep.holla@arm.com>
>>> Cc: Rob Herring <robh@kernel.org>
>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>> ---
>>>   .../devicetree/bindings/arm/coresight.txt          | 29 ++++++++++---
>>>   drivers/hwtracing/coresight/of_coresight.c         | 49
>>> +++++++++++++++++-----
>>>   2 files changed, 62 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt
>>> b/Documentation/devicetree/bindings/arm/coresight.txt
>>> index ed6b555..bf75ab3 100644
>>> --- a/Documentation/devicetree/bindings/arm/coresight.txt
>>> +++ b/Documentation/devicetree/bindings/arm/coresight.txt
>>> @@ -108,8 +108,13 @@ following properties to uniquely identify the
>>> connection details.
>>>         "slave-mode"
>
>
>
>>>         };
>>
>>
>> For the binding part:
>> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>>
>>> diff --git a/drivers/hwtracing/coresight/of_coresight.c
>>> b/drivers/hwtracing/coresight/of_coresight.c
>>> index d01a9ce..d23d7dd 100644
>>> --- a/drivers/hwtracing/coresight/of_coresight.c
>>> +++ b/drivers/hwtracing/coresight/of_coresight.c
>
>
> ...
>
>>> +/*
>>>    * of_coresight_parse_endpoint : Parse the given output endpoint @ep
>>>    * and fill the connection information in *@pconn.
>>>    *
>>> @@ -109,11 +134,11 @@ EXPORT_SYMBOL_GPL(of_coresight_get_cpu);
>>>    *     0      - If the parsing completed without any fatal errors.
>
>
> Please note the return value description here. Further comments below.
>
>>>    *    -Errno  - Fatal error, abort the scanning.
>>>    */
>>> -static int of_coresight_parse_endpoint(struct device_node *ep,
>>> +static int of_coresight_parse_endpoint(struct device *dev,
>>> +                                      struct device_node *ep,
>>>                                        struct coresight_connection
>>> **pconn)
>>>   {
>>> -       int ret = 0;
>>> -       struct of_endpoint endpoint, rendpoint;
>>> +       int ret = 0, local_port, child_port;
>>>         struct device_node *rparent = NULL;
>>>         struct device_node *rep = NULL;
>>>         struct device *rdev = NULL;
>>> @@ -128,7 +153,8 @@ static int of_coresight_parse_endpoint(struct
>>> device_node *ep,
>>>                         break;
>>>                 /* Parse the local port details */
>>> -               if (of_graph_parse_endpoint(ep, &endpoint))
>>> +               local_port = of_coresight_endpoint_get_port_id(dev, ep);
>>> +               if (local_port < 0)
>>>                         break;
>>>                 /*
>>>                  * Get a handle on the remote endpoint and the device it
>>> is
>>> @@ -140,9 +166,6 @@ static int of_coresight_parse_endpoint(struct
>>> device_node *ep,
>>>                 rparent = of_graph_get_port_parent(rep);
>>>                 if (!rparent)
>>>                         break;
>>> -               if (of_graph_parse_endpoint(rep, &rendpoint))
>>> -                       break;
>>> -
>>>                 /* If the remote device is not available, defer probing
>>> */
>>>                 rdev = of_coresight_get_endpoint_device(rparent);
>>>                 if (!rdev) {
>>> @@ -150,9 +173,15 @@ static int of_coresight_parse_endpoint(struct
>>> device_node *ep,
>>>                         break;
>>>                 }
>>>   -             conn->outport = endpoint.port;
>>> +               child_port = of_coresight_endpoint_get_port_id(rdev,
>>> rep);
>>> +               if (child_port < 0) {
>>> +                       ret = 0;
>>
>>
>> Why returning '0' on an error condition?  Same for 'local_port' above.
>>
>
> If we are unable to parse a port, we can simply ignore the port and
> continue, which
> is what we have today with the existing code. I didn't change it and still
> think
> it is the best effort thing. We could spit a warning for such cases, if
> really needed.
> Also, the parsing code almost never fails at the moment. If it fails to find
> "reg" field,
> it is assumed to be '0'. Either way ignoring it seems harmless. That said I
> am open
> to suggestions.

Looking at the original code I remember not mandating enpoints to be
valid for debugging purposes.  That certainly helps when building up a
device tree file but also has the side effect of silently overlooking
specification problems.  Fortunately the revamping you did on that
part of the code makes it very easy to change that, something I think
we should take advantage of (it can only lead to positive scenarios
where defective specifications get pointed out).

That being said and because the original behaviour is just as
permissive, you can leave as is.

Thanks,
Mathieu

>
> Cheers
> Suzuki

^ permalink raw reply

* [PATCH] drm/meson: Fix an un-handled error path in 'meson_drv_bind_master()'
From: Christophe JAILLET @ 2018-06-11 16:53 UTC (permalink / raw)
  To: linux-arm-kernel

If 'platform_get_resource_byname()' fails, we should release some resources
before leaving, as already done in the other error handling path of the
function.

Fixes: acaa3f13b8dd ("drm/meson: Fix potential NULL dereference in meson_drv_bind_master()")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/gpu/drm/meson/meson_drv.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index 32b1a6cdecfc..d3443125e661 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -197,8 +197,10 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
 	priv->io_base = regs;
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "hhi");
-	if (!res)
-		return -EINVAL;
+	if (!res) {
+		ret = -EINVAL;
+		goto free_drm;
+	}
 	/* Simply ioremap since it may be a shared register zone */
 	regs = devm_ioremap(dev, res->start, resource_size(res));
 	if (!regs) {
@@ -215,8 +217,10 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
 	}
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dmc");
-	if (!res)
-		return -EINVAL;
+	if (!res) {
+		ret = -EINVAL;
+		goto free_drm;
+	}
 	/* Simply ioremap since it may be a shared register zone */
 	regs = devm_ioremap(dev, res->start, resource_size(res));
 	if (!regs) {
-- 
2.17.0

^ permalink raw reply related

* [PATCH 08/20] coresight: dts: Cleanup device tree graph bindings
From: Suzuki K Poulose @ 2018-06-11 16:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CANLsYkwGmUpHKRZYJ=pGbEKOAGT6O=+u0_mpAJOyoONuqpXO4g@mail.gmail.com>

On 11/06/18 17:52, Mathieu Poirier wrote:
> On 11 June 2018 at 03:22, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote:
>> On 08/06/18 22:22, Mathieu Poirier wrote:
>>>
>>> On Tue, Jun 05, 2018 at 10:43:19PM +0100, Suzuki K Poulose wrote:
>>>>
>>>> The coresight drivers relied on default bindings for graph
>>>> in DT, while reusing the "reg" field of the "ports" to indicate
>>>> the actual hardware port number for the connections. However,
>>>> with the rules getting stricter w.r.t to the address mismatch
>>>> with the label, it is no longer possible to use the port address
>>>> field for the hardware port number. Hence, we add an explicit
>>>> property to denote the hardware port number, "coresight,hwid"
>>>> which must be specified for each "endpoint".
>>>>
>>>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>>>> Cc: Sudeep Holla <sudeep.holla@arm.com>
>>>> Cc: Rob Herring <robh@kernel.org>
>>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>>> ---
>>>>    .../devicetree/bindings/arm/coresight.txt          | 29 ++++++++++---
>>>>    drivers/hwtracing/coresight/of_coresight.c         | 49
>>>> +++++++++++++++++-----
>>>>    2 files changed, 62 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt
>>>> b/Documentation/devicetree/bindings/arm/coresight.txt
>>>> index ed6b555..bf75ab3 100644
>>>> --- a/Documentation/devicetree/bindings/arm/coresight.txt
>>>> +++ b/Documentation/devicetree/bindings/arm/coresight.txt
>>>> @@ -108,8 +108,13 @@ following properties to uniquely identify the
>>>> connection details.
>>>>          "slave-mode"
>>
>>
>>
>>>>          };
>>>
>>>
>>> For the binding part:
>>> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>

...

>>>> @@ -140,9 +166,6 @@ static int of_coresight_parse_endpoint(struct
>>>> device_node *ep,
>>>>                  rparent = of_graph_get_port_parent(rep);
>>>>                  if (!rparent)
>>>>                          break;
>>>> -               if (of_graph_parse_endpoint(rep, &rendpoint))
>>>> -                       break;
>>>> -
>>>>                  /* If the remote device is not available, defer probing
>>>> */
>>>>                  rdev = of_coresight_get_endpoint_device(rparent);
>>>>                  if (!rdev) {
>>>> @@ -150,9 +173,15 @@ static int of_coresight_parse_endpoint(struct
>>>> device_node *ep,
>>>>                          break;
>>>>                  }
>>>>    -             conn->outport = endpoint.port;
>>>> +               child_port = of_coresight_endpoint_get_port_id(rdev,
>>>> rep);
>>>> +               if (child_port < 0) {
>>>> +                       ret = 0;
>>>
>>>
>>> Why returning '0' on an error condition?  Same for 'local_port' above.
>>>
>>
>> If we are unable to parse a port, we can simply ignore the port and
>> continue, which
>> is what we have today with the existing code. I didn't change it and still
>> think
>> it is the best effort thing. We could spit a warning for such cases, if
>> really needed.
>> Also, the parsing code almost never fails at the moment. If it fails to find
>> "reg" field,
>> it is assumed to be '0'. Either way ignoring it seems harmless. That said I
>> am open
>> to suggestions.
> 
> Looking at the original code I remember not mandating enpoints to be
> valid for debugging purposes.  That certainly helps when building up a
> device tree file but also has the side effect of silently overlooking
> specification problems.  Fortunately the revamping you did on that
> part of the code makes it very easy to change that, something I think
> we should take advantage of (it can only lead to positive scenarios
> where defective specifications get pointed out).
> 
> That being said and because the original behaviour is just as
> permissive, you can leave as is.

Thanks. So can I assume the Reviewed-by applies for the code now ?

Suzuki

^ permalink raw reply

* [PATCH v7 2/2] regulator: add QCOM RPMh regulator driver
From: Matthias Kaehlcke @ 2018-06-11 17:19 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <36e91ad9edeeee2adfe7a4d92675a1d6950f5002.1528498807.git.collinsd@codeaurora.org>

On Fri, Jun 08, 2018 at 04:44:15PM -0700, David Collins wrote:
> Add the QCOM RPMh regulator driver to manage PMIC regulators
> which are controlled via RPMh on some Qualcomm Technologies, Inc.
> SoCs.  RPMh is a hardware block which contains several
> accelerators which are used to manage various hardware resources
> that are shared between the processors of the SoC.  The final
> hardware state of a regulator is determined within RPMh by
> performing max aggregation of the requests made by all of the
> processors.
> 
> Add support for PMIC regulator control via the voltage regulator
> manager (VRM) and oscillator buffer (XOB) RPMh accelerators.
> VRM supports manipulation of enable state, voltage, and mode.
> XOB supports manipulation of enable state.
> 
> Signed-off-by: David Collins <collinsd@codeaurora.org>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> ---
>  drivers/regulator/Kconfig               |   9 +
>  drivers/regulator/Makefile              |   1 +
>  drivers/regulator/qcom-rpmh-regulator.c | 753 ++++++++++++++++++++++++++++++++
>  3 files changed, 763 insertions(+)
>  create mode 100644 drivers/regulator/qcom-rpmh-regulator.c

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox