* [PATCH] iio: stm32 trigger: Add support for TRGO2 triggers
From: Benjamin Gaignard @ 2017-04-26 8:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1493194662-14501-1-git-send-email-fabrice.gasnier@st.com>
2017-04-26 10:17 GMT+02:00 Fabrice Gasnier <fabrice.gasnier@st.com>:
> Add support for TRGO2 trigger that can be found on STM32F7.
> Add additional master modes supported by TRGO2.
> Register additional "tim[1/8]_trgo2" triggers for timer1 & timer8.
> Detect TRGO2 timer capability (master mode selection 2).
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> ---
> .../ABI/testing/sysfs-bus-iio-timer-stm32 | 15 +++
> drivers/iio/trigger/stm32-timer-trigger.c | 113 ++++++++++++++++++---
> include/linux/iio/timer/stm32-timer-trigger.h | 2 +
> include/linux/mfd/stm32-timers.h | 2 +
> 4 files changed, 118 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
> index 230020e..47647b4 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
> @@ -16,6 +16,21 @@ Description:
> - "OC2REF" : OC2REF signal is used as trigger output.
> - "OC3REF" : OC3REF signal is used as trigger output.
> - "OC4REF" : OC4REF signal is used as trigger output.
> + Additional modes (on TRGO2 only):
> + - "OC5REF" : OC5REF signal is used as trigger output.
> + - "OC6REF" : OC6REF signal is used as trigger output.
> + - "compare_pulse_OC4REF":
> + OC4REF rising or falling edges generate pulses.
> + - "compare_pulse_OC6REF":
> + OC6REF rising or falling edges generate pulses.
> + - "compare_pulse_OC4REF_r_or_OC6REF_r":
> + OC4REF or OC6REF rising edges generate pulses.
> + - "compare_pulse_OC4REF_r_or_OC6REF_f":
> + OC4REF rising or OC6REF falling edges generate pulses.
> + - "compare_pulse_OC5REF_r_or_OC6REF_r":
> + OC5REF or OC6REF rising edges generate pulses.
> + - "compare_pulse_OC5REF_r_or_OC6REF_f":
> + OC5REF rising or OC6REF falling edges generate pulses.
>
> What: /sys/bus/iio/devices/triggerX/master_mode
> KernelVersion: 4.11
> diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
> index 0f1a2cf..a0031b7 100644
> --- a/drivers/iio/trigger/stm32-timer-trigger.c
> +++ b/drivers/iio/trigger/stm32-timer-trigger.c
> @@ -14,19 +14,19 @@
> #include <linux/module.h>
> #include <linux/platform_device.h>
>
> -#define MAX_TRIGGERS 6
> +#define MAX_TRIGGERS 7
> #define MAX_VALIDS 5
>
> /* List the triggers created by each timer */
> static const void *triggers_table[][MAX_TRIGGERS] = {
> - { TIM1_TRGO, TIM1_CH1, TIM1_CH2, TIM1_CH3, TIM1_CH4,},
> + { TIM1_TRGO, TIM1_TRGO2, TIM1_CH1, TIM1_CH2, TIM1_CH3, TIM1_CH4,},
> { TIM2_TRGO, TIM2_CH1, TIM2_CH2, TIM2_CH3, TIM2_CH4,},
> { TIM3_TRGO, TIM3_CH1, TIM3_CH2, TIM3_CH3, TIM3_CH4,},
> { TIM4_TRGO, TIM4_CH1, TIM4_CH2, TIM4_CH3, TIM4_CH4,},
> { TIM5_TRGO, TIM5_CH1, TIM5_CH2, TIM5_CH3, TIM5_CH4,},
> { TIM6_TRGO,},
> { TIM7_TRGO,},
> - { TIM8_TRGO, TIM8_CH1, TIM8_CH2, TIM8_CH3, TIM8_CH4,},
> + { TIM8_TRGO, TIM8_TRGO2, TIM8_CH1, TIM8_CH2, TIM8_CH3, TIM8_CH4,},
> { TIM9_TRGO, TIM9_CH1, TIM9_CH2,},
> { }, /* timer 10 */
> { }, /* timer 11 */
> @@ -56,9 +56,16 @@ struct stm32_timer_trigger {
> u32 max_arr;
> const void *triggers;
> const void *valids;
> + bool has_trgo2;
> };
>
> +static bool stm32_timer_is_trgo2_name(const char *name)
> +{
> + return !!strstr(name, "trgo2");
> +}
> +
> static int stm32_timer_start(struct stm32_timer_trigger *priv,
> + struct iio_trigger *trig,
> unsigned int frequency)
> {
> unsigned long long prd, div;
> @@ -102,7 +109,12 @@ static int stm32_timer_start(struct stm32_timer_trigger *priv,
> regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_ARPE, TIM_CR1_ARPE);
>
> /* Force master mode to update mode */
> - regmap_update_bits(priv->regmap, TIM_CR2, TIM_CR2_MMS, 0x20);
> + if (stm32_timer_is_trgo2_name(trig->name))
> + regmap_update_bits(priv->regmap, TIM_CR2, TIM_CR2_MMS2,
> + 0x2 << TIM_CR2_MMS2_SHIFT);
> + else
> + regmap_update_bits(priv->regmap, TIM_CR2, TIM_CR2_MMS,
> + 0x2 << TIM_CR2_MMS_SHIFT);
>
> /* Make sure that registers are updated */
> regmap_update_bits(priv->regmap, TIM_EGR, TIM_EGR_UG, TIM_EGR_UG);
> @@ -150,7 +162,7 @@ static ssize_t stm32_tt_store_frequency(struct device *dev,
> if (freq == 0) {
> stm32_timer_stop(priv);
> } else {
> - ret = stm32_timer_start(priv, freq);
> + ret = stm32_timer_start(priv, trig, freq);
> if (ret)
> return ret;
> }
> @@ -183,6 +195,9 @@ static IIO_DEV_ATTR_SAMP_FREQ(0660,
> stm32_tt_read_frequency,
> stm32_tt_store_frequency);
>
> +#define MASTER_MODE_MAX 7
> +#define MASTER_MODE2_MAX 15
> +
> static char *master_mode_table[] = {
> "reset",
> "enable",
> @@ -191,7 +206,16 @@ static IIO_DEV_ATTR_SAMP_FREQ(0660,
> "OC1REF",
> "OC2REF",
> "OC3REF",
> - "OC4REF"
> + "OC4REF",
> + /* Master mode selection 2 only */
> + "OC5REF",
> + "OC6REF",
> + "compare_pulse_OC4REF",
> + "compare_pulse_OC6REF",
> + "compare_pulse_OC4REF_r_or_OC6REF_r",
> + "compare_pulse_OC4REF_r_or_OC6REF_f",
> + "compare_pulse_OC5REF_r_or_OC6REF_r",
> + "compare_pulse_OC5REF_r_or_OC6REF_f",
> };
>
> static ssize_t stm32_tt_show_master_mode(struct device *dev,
> @@ -199,10 +223,15 @@ static ssize_t stm32_tt_show_master_mode(struct device *dev,
> char *buf)
> {
> struct stm32_timer_trigger *priv = dev_get_drvdata(dev);
> + struct iio_trigger *trig = to_iio_trigger(dev);
> u32 cr2;
>
> regmap_read(priv->regmap, TIM_CR2, &cr2);
> - cr2 = (cr2 & TIM_CR2_MMS) >> TIM_CR2_MMS_SHIFT;
> +
> + if (stm32_timer_is_trgo2_name(trig->name))
> + cr2 = (cr2 & TIM_CR2_MMS2) >> TIM_CR2_MMS2_SHIFT;
> + else
> + cr2 = (cr2 & TIM_CR2_MMS) >> TIM_CR2_MMS_SHIFT;
>
> return snprintf(buf, PAGE_SIZE, "%s\n", master_mode_table[cr2]);
> }
> @@ -212,13 +241,25 @@ static ssize_t stm32_tt_store_master_mode(struct device *dev,
> const char *buf, size_t len)
> {
> struct stm32_timer_trigger *priv = dev_get_drvdata(dev);
> + struct iio_trigger *trig = to_iio_trigger(dev);
> + u32 mask, shift, master_mode_max;
> int i;
>
> - for (i = 0; i < ARRAY_SIZE(master_mode_table); i++) {
> + if (stm32_timer_is_trgo2_name(trig->name)) {
> + mask = TIM_CR2_MMS2;
> + shift = TIM_CR2_MMS2_SHIFT;
> + master_mode_max = MASTER_MODE2_MAX;
> + } else {
> + mask = TIM_CR2_MMS;
> + shift = TIM_CR2_MMS_SHIFT;
> + master_mode_max = MASTER_MODE_MAX;
> + }
> +
> + for (i = 0; i <= master_mode_max; i++) {
> if (!strncmp(master_mode_table[i], buf,
> strlen(master_mode_table[i]))) {
> - regmap_update_bits(priv->regmap, TIM_CR2,
> - TIM_CR2_MMS, i << TIM_CR2_MMS_SHIFT);
> + regmap_update_bits(priv->regmap, TIM_CR2, mask,
> + i << shift);
> /* Make sure that registers are updated */
> regmap_update_bits(priv->regmap, TIM_EGR,
> TIM_EGR_UG, TIM_EGR_UG);
> @@ -229,8 +270,31 @@ static ssize_t stm32_tt_store_master_mode(struct device *dev,
> return -EINVAL;
> }
>
> -static IIO_CONST_ATTR(master_mode_available,
> - "reset enable update compare_pulse OC1REF OC2REF OC3REF OC4REF");
> +static ssize_t stm32_tt_show_master_mode_avail(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct iio_trigger *trig = to_iio_trigger(dev);
> + unsigned int i, master_mode_max;
> + size_t len = 0;
> +
> + if (stm32_timer_is_trgo2_name(trig->name))
> + master_mode_max = MASTER_MODE2_MAX;
> + else
> + master_mode_max = MASTER_MODE_MAX;
> +
> + for (i = 0; i <= master_mode_max; i++)
> + len += scnprintf(buf + len, PAGE_SIZE - len,
> + "%s ", master_mode_table[i]);
> +
> + /* replace trailing space by newline */
> + buf[len - 1] = '\n';
> +
> + return len;
> +}
> +
> +static IIO_DEVICE_ATTR(master_mode_available, 0444,
> + stm32_tt_show_master_mode_avail, NULL, 0);
>
> static IIO_DEVICE_ATTR(master_mode, 0660,
> stm32_tt_show_master_mode,
> @@ -240,7 +304,7 @@ static IIO_DEVICE_ATTR(master_mode, 0660,
> static struct attribute *stm32_trigger_attrs[] = {
> &iio_dev_attr_sampling_frequency.dev_attr.attr,
> &iio_dev_attr_master_mode.dev_attr.attr,
> - &iio_const_attr_master_mode_available.dev_attr.attr,
> + &iio_dev_attr_master_mode_available.dev_attr.attr,
> NULL,
> };
>
> @@ -264,6 +328,12 @@ static int stm32_setup_iio_triggers(struct stm32_timer_trigger *priv)
>
> while (cur && *cur) {
> struct iio_trigger *trig;
> + bool cur_is_trgo2 = stm32_timer_is_trgo2_name(*cur);
> +
> + if (cur_is_trgo2 && !priv->has_trgo2) {
> + cur++;
> + continue;
> + }
>
> trig = devm_iio_trigger_alloc(priv->dev, "%s", *cur);
> if (!trig)
> @@ -277,7 +347,7 @@ static int stm32_setup_iio_triggers(struct stm32_timer_trigger *priv)
> * should only be available on trgo trigger which
> * is always the first in the list.
> */
> - if (cur == priv->triggers)
> + if (cur == priv->triggers || cur_is_trgo2)
> trig->dev.groups = stm32_trigger_attr_groups;
>
> iio_trigger_set_drvdata(trig, priv);
> @@ -584,6 +654,20 @@ bool is_stm32_timer_trigger(struct iio_trigger *trig)
> }
> EXPORT_SYMBOL(is_stm32_timer_trigger);
>
> +static void stm32_timer_detect_trgo2(struct stm32_timer_trigger *priv)
> +{
> + u32 val;
> +
> + /*
> + * Master mode selection 2 bits can only be written and read back when
> + * timer supports it.
> + */
> + regmap_update_bits(priv->regmap, TIM_CR2, TIM_CR2_MMS2, TIM_CR2_MMS2);
> + regmap_read(priv->regmap, TIM_CR2, &val);
> + regmap_update_bits(priv->regmap, TIM_CR2, TIM_CR2_MMS2, 0);
> + priv->has_trgo2 = !!val;
> +}
> +
> static int stm32_timer_trigger_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -614,6 +698,7 @@ static int stm32_timer_trigger_probe(struct platform_device *pdev)
> priv->max_arr = ddata->max_arr;
> priv->triggers = triggers_table[index];
> priv->valids = valids_table[index];
> + stm32_timer_detect_trgo2(priv);
>
> ret = stm32_setup_iio_triggers(priv);
> if (ret)
> diff --git a/include/linux/iio/timer/stm32-timer-trigger.h b/include/linux/iio/timer/stm32-timer-trigger.h
> index 55535ae..fa7d786 100644
> --- a/include/linux/iio/timer/stm32-timer-trigger.h
> +++ b/include/linux/iio/timer/stm32-timer-trigger.h
> @@ -10,6 +10,7 @@
> #define _STM32_TIMER_TRIGGER_H_
>
> #define TIM1_TRGO "tim1_trgo"
> +#define TIM1_TRGO2 "tim1_trgo2"
> #define TIM1_CH1 "tim1_ch1"
> #define TIM1_CH2 "tim1_ch2"
> #define TIM1_CH3 "tim1_ch3"
> @@ -44,6 +45,7 @@
> #define TIM7_TRGO "tim7_trgo"
>
> #define TIM8_TRGO "tim8_trgo"
> +#define TIM8_TRGO2 "tim8_trgo2"
> #define TIM8_CH1 "tim8_ch1"
> #define TIM8_CH2 "tim8_ch2"
> #define TIM8_CH3 "tim8_ch3"
> diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
> index 4a0abbc..ce7346e 100644
> --- a/include/linux/mfd/stm32-timers.h
> +++ b/include/linux/mfd/stm32-timers.h
> @@ -34,6 +34,7 @@
> #define TIM_CR1_DIR BIT(4) /* Counter Direction */
> #define TIM_CR1_ARPE BIT(7) /* Auto-reload Preload Ena */
> #define TIM_CR2_MMS (BIT(4) | BIT(5) | BIT(6)) /* Master mode selection */
> +#define TIM_CR2_MMS2 GENMASK(23, 20) /* Master mode selection 2 */
> #define TIM_SMCR_SMS (BIT(0) | BIT(1) | BIT(2)) /* Slave mode selection */
> #define TIM_SMCR_TS (BIT(4) | BIT(5) | BIT(6)) /* Trigger selection */
> #define TIM_DIER_UIE BIT(0) /* Update interrupt */
> @@ -60,6 +61,7 @@
>
> #define MAX_TIM_PSC 0xFFFF
> #define TIM_CR2_MMS_SHIFT 4
> +#define TIM_CR2_MMS2_SHIFT 20
> #define TIM_SMCR_TS_SHIFT 4
> #define TIM_BDTR_BKF_MASK 0xF
> #define TIM_BDTR_BKF_SHIFT 16
> --
> 1.9.1
>
Acked-by: Benjamin Gaiganrd <benjamin.gaignard@linaro.org>
^ permalink raw reply
* [PATCH v5 02/22] KVM: arm/arm64: Add GICV3 pending table save API documentation
From: Dr. David Alan Gilbert @ 2017-04-26 8:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAFEAcA-u-sH+v-Pnr_3gRC3iSJP0ZK68+GqkDzmi3xe1-B2WAQ@mail.gmail.com>
* Peter Maydell (peter.maydell at linaro.org) wrote:
> On 26 April 2017 at 09:26, Auger Eric <eric.auger@redhat.com> wrote:
> > On 25/04/2017 12:43, Peter Maydell wrote:
> >> When does the -EFAULT return happen? (if the guest points GITS_BASER<n>
> >> etc at invalid memory, presumably?)
> >
> > Yes that's correct, when GICR_PENDBASER contains a bad GPA.
> >
> >> How does the QEMU migration code
> >> handle this case? Failing migration because the guest has done something
> >> silly doesn't seem too palatable, but trying to avoid that could be
> >> more effort than an obscure corner case really merits.
> >
> > The kvm_device_access will cause an abort() as for other errors returned
> > by kvm_device_ioctl().
>
> That's pretty nasty. Guests shouldn't be able to provoke QEMU
> into abort()ing, ideally. We don't necessarily have to produce
> a successful migration, but we should at least fail it cleanly.
Yes, no abort()'s during migration due to guest behaviour.
They always end up coming back around to being filed as migration
bugs and people worry why they've got cores.
Ideally log a message into stderr to say that the guest state
is inconsistent so that when someone comes to debug it then they
can see it's obvious.
Dave
> thanks
> -- PMM
--
Dr. David Alan Gilbert / dgilbert at redhat.com / Manchester, UK
^ permalink raw reply
* [PATCH v5 02/22] KVM: arm/arm64: Add GICV3 pending table save API documentation
From: Peter Maydell @ 2017-04-26 8:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <a5ab9181-8398-f62b-12f1-032961b12c28@redhat.com>
On 26 April 2017 at 09:26, Auger Eric <eric.auger@redhat.com> wrote:
> On 25/04/2017 12:43, Peter Maydell wrote:
>> When does the -EFAULT return happen? (if the guest points GITS_BASER<n>
>> etc at invalid memory, presumably?)
>
> Yes that's correct, when GICR_PENDBASER contains a bad GPA.
>
>> How does the QEMU migration code
>> handle this case? Failing migration because the guest has done something
>> silly doesn't seem too palatable, but trying to avoid that could be
>> more effort than an obscure corner case really merits.
>
> The kvm_device_access will cause an abort() as for other errors returned
> by kvm_device_ioctl().
That's pretty nasty. Guests shouldn't be able to provoke QEMU
into abort()ing, ideally. We don't necessarily have to produce
a successful migration, but we should at least fail it cleanly.
thanks
-- PMM
^ permalink raw reply
* [RFC] usb: chipidea: set dma_ops for the created ci_hdrc platform_device
From: Jisheng Zhang @ 2017-04-26 8:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAK8P3a09fJCexChnoEyBpLK17rHaQTz9G+7iZWRocZGykyxsWA@mail.gmail.com>
On Tue, 25 Apr 2017 13:09:27 +0200 Arnd Bergmann wrote:
> On Tue, Apr 25, 2017 at 12:01 PM, Jisheng Zhang <jszhang@marvell.com> wrote:
> > Hi all,
> >
> > After commit 1dccb598df549 ("arm64: simplify dma_get_ops"), the chipidea
> > driver can't work any more on Marvell Berlin arm64 platforms, the reason
> > is the created ci_hdrc platform_device's dma_ops is dummy_dma_ops, so all
> > dma related operations will fail. The fix I can think of would be something
> > as below:
> >
> > And I noticed that dwc3 has the same issue[1], and as pointed out in its
> > discussion, the patch can't fix None-DT platforms, so could you please
> > guide me what's the proper fix which can be mainlined?
>
> I think the right solution is:
>
> - Set the "sysdev" pointer tin the USB device o the device structure that
> was created by DT or the legacy board file and remove the manual
> setting of dma_mask, parms and the dma_configure. This should
> make everything work as expected in case of DT
>
oh yeah! I see a proper fix in linux-next tree, thanks for fixing it.
^ permalink raw reply
* [PATCH] usb: chipidea: properly handle host or gadget initialization failure
From: Peter Chen @ 2017-04-26 8:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170425094311.361-1-jszhang@marvell.com>
On Tue, Apr 25, 2017 at 05:43:11PM +0800, Jisheng Zhang wrote:
> If ci_hdrc_host_init() or ci_hdrc_gadget_init() returns error and the
> error != -ENXIO, as Peter pointed out, "it stands for initialization
> for host or gadget has failed", so we'd better return failure rather
> continue.
>
> Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
> ---
> drivers/usb/chipidea/core.c | 25 +++++++++++++++++++------
> 1 file changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 79ad8e91632e..047afdbb7049 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -930,20 +930,28 @@ static int ci_hdrc_probe(struct platform_device *pdev)
> /* initialize role(s) before the interrupt is requested */
> if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {
> ret = ci_hdrc_host_init(ci);
> - if (ret)
> - dev_info(dev, "doesn't support host\n");
> + if (ret) {
> + if (ret == -ENXIO)
> + dev_info(dev, "doesn't support host\n");
> + else
> + goto deinit_phy;
> + }
> }
>
> if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_PERIPHERAL) {
> ret = ci_hdrc_gadget_init(ci);
> - if (ret)
> - dev_info(dev, "doesn't support gadget\n");
> + if (ret) {
> + if (ret == -ENXIO)
> + dev_info(dev, "doesn't support gadget\n");
> + else
> + goto deinit_host;
> + }
> }
>
> if (!ci->roles[CI_ROLE_HOST] && !ci->roles[CI_ROLE_GADGET]) {
> dev_err(dev, "no supported roles\n");
> ret = -ENODEV;
> - goto deinit_phy;
> + goto deinit_gadget;
> }
>
> if (ci->is_otg && ci->roles[CI_ROLE_GADGET]) {
> @@ -1013,7 +1021,12 @@ static int ci_hdrc_probe(struct platform_device *pdev)
> return 0;
>
> stop:
> - ci_role_destroy(ci);
> + if (ci->is_otg)
> + ci_hdrc_otg_destroy(ci);
> +deinit_gadget:
> + ci_hdrc_gadget_destroy(ci);
> +deinit_host:
> + ci_hdrc_host_destroy(ci);
> deinit_phy:
> ci_usb_phy_exit(ci);
> ulpi_exit:
> --
> 2.11.0
>
You change is ok, there are more things need to change for this issue,
would you mind adding below changes to your patch, and send v2. Below
changes are based on your changes.
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index c55bcdd..4cad8a9 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -818,7 +818,7 @@ static inline void ci_role_destroy(struct ci_hdrc *ci)
{
ci_hdrc_gadget_destroy(ci);
ci_hdrc_host_destroy(ci);
- if (ci->is_otg)
+ if (ci->is_otg && ci->roles[CI_ROLE_GADGET])
ci_hdrc_otg_destroy(ci);
}
@@ -1005,7 +1005,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
ret = ci_hdrc_otg_init(ci);
if (ret) {
dev_err(dev, "init otg fails, ret = %d\n", ret);
- goto stop;
+ goto deinit_gadget;
}
}
@@ -1075,7 +1075,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
remove_debug:
dbg_remove_files(ci);
stop:
- if (ci->is_otg)
+ if (ci->is_otg && ci->roles[CI_ROLE_GADGET])
ci_hdrc_otg_destroy(ci);
deinit_gadget:
ci_hdrc_gadget_destroy(ci);
--
Best Regards,
Peter Chen
^ permalink raw reply related
* [PATCH v5 02/22] KVM: arm/arm64: Add GICV3 pending table save API documentation
From: Auger Eric @ 2017-04-26 8:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAFEAcA_0N9Jx5ym+=jjmxAzSaXPjt-m+gW1cq90of8Qhw3LOzg@mail.gmail.com>
Hi Peter,
On 25/04/2017 12:43, Peter Maydell wrote:
> On 14 April 2017 at 11:15, Eric Auger <eric.auger@redhat.com> wrote:
>> Add description for how to save GICV3 LPI pending bit into
>> guest RAM pending tables.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>> v5: creation
>> ---
>> Documentation/virtual/kvm/devices/arm-vgic-v3.txt | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/virtual/kvm/devices/arm-vgic-v3.txt b/Documentation/virtual/kvm/devices/arm-vgic-v3.txt
>> index c1a2461..9293b45 100644
>> --- a/Documentation/virtual/kvm/devices/arm-vgic-v3.txt
>> +++ b/Documentation/virtual/kvm/devices/arm-vgic-v3.txt
>> @@ -167,11 +167,17 @@ Groups:
>> KVM_DEV_ARM_VGIC_CTRL_INIT
>> request the initialization of the VGIC, no additional parameter in
>> kvm_device_attr.addr.
>> + KVM_DEV_ARM_VGIC_SAVE_PENDING_TABLES
>> + save all LPI pending bits into guest RAM pending tables.
>> +
>> + The first kB of the pending table is not altered by this operation.
>> Errors:
>> -ENXIO: VGIC not properly configured as required prior to calling
>> this attribute
>> -ENODEV: no online VCPU
>> -ENOMEM: memory shortage when allocating vgic internal data
>> + -EFAULT: Invalid guest ram access
>> + -EBUSY: One or more VCPUS are running
>>
>>
>> KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO
>> --
>> 2.5.5
>
> When does the -EFAULT return happen? (if the guest points GITS_BASER<n>
> etc at invalid memory, presumably?)
Yes that's correct, when GICR_PENDBASER contains a bad GPA.
How does the QEMU migration code
> handle this case? Failing migration because the guest has done something
> silly doesn't seem too palatable, but trying to avoid that could be
> more effort than an obscure corner case really merits.
The kvm_device_access will cause an abort() as for other errors returned
by kvm_device_ioctl().
Thanks
Eric
>
> thanks
> -- PMM
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* [PATCH] usb: chipidea: udc: fix NULL pointer dereference if udc_start failed
From: Jisheng Zhang @ 2017-04-26 8:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <0ac4bc53-dd26-0b90-c84e-a94d01a6d5e4@i2se.com>
On Tue, 25 Apr 2017 17:21:59 +0200
Stefan Wahren <stefan.wahren@i2se.com> wrote:
> Am 25.04.2017 um 11:20 schrieb Peter Chen:
> >
> >>>> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> >>>> index f88e9157fad0..60a786c87c06 100644
> >>>> --- a/drivers/usb/chipidea/udc.c
> >>>> +++ b/drivers/usb/chipidea/udc.c
> >>>> @@ -1984,6 +1984,7 @@ static void udc_id_switch_for_host(struct
> >>>> ci_hdrc *ci) int ci_hdrc_gadget_init(struct ci_hdrc *ci) {
> >>>> struct ci_role_driver *rdrv;
> >>>> + int ret;
> >>>>
> >>>> if (!hw_read(ci, CAP_DCCPARAMS, DCCPARAMS_DC))
> >>>> return -ENXIO;
> >>>> @@ -1996,7 +1997,10 @@ int ci_hdrc_gadget_init(struct ci_hdrc *ci)
> >>>> rdrv->stop = udc_id_switch_for_host;
> >>>> rdrv->irq = udc_irq;
> >>>> rdrv->name = "gadget";
> >>>> - ci->roles[CI_ROLE_GADGET] = rdrv;
> >>>>
> >>>> - return udc_start(ci);
> >>>> + ret = udc_start(ci);
> >>>> + if (!ret)
> >>>> + ci->roles[CI_ROLE_GADGET] = rdrv;
> >>>> +
> >>>> + return ret;
> >>>> }
> >>>> --
> >>> Thanks for fixing it. In fact, we'd better return failure if ret &&
> >>> ret != -ENXIO at probe, it stands for initialization for host or
> >>> gadget has failed.
> >>>
> >> I got your meaning. I'll cook v2. I don't have preference, since either one can fix the
> >> issue.
> >>
> > Both are needed, you don't need to send this one again. Only a new one, thanks.
>
> I'm not sure how easy it is to reproduce the issue.
It's easy to reproduce it (100%) on arm64 platforms after commit
1dccb598df549 ("arm64: simplify dma_get_ops"). This commit could
make all dma related operations failed, then udc_start() would fail
with -ENOMEM.
On other platforms, it's not easy.
>
> Shouldn't make a Fixes tag sense at least?
maybe 3f124d233e97 ("usb: chipidea: add role init and destroy APIs"
^ permalink raw reply
* [PATCH] iio: stm32 trigger: Add support for TRGO2 triggers
From: Fabrice Gasnier @ 2017-04-26 8:17 UTC (permalink / raw)
To: linux-arm-kernel
Add support for TRGO2 trigger that can be found on STM32F7.
Add additional master modes supported by TRGO2.
Register additional "tim[1/8]_trgo2" triggers for timer1 & timer8.
Detect TRGO2 timer capability (master mode selection 2).
Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
.../ABI/testing/sysfs-bus-iio-timer-stm32 | 15 +++
drivers/iio/trigger/stm32-timer-trigger.c | 113 ++++++++++++++++++---
include/linux/iio/timer/stm32-timer-trigger.h | 2 +
include/linux/mfd/stm32-timers.h | 2 +
4 files changed, 118 insertions(+), 14 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
index 230020e..47647b4 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
+++ b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
@@ -16,6 +16,21 @@ Description:
- "OC2REF" : OC2REF signal is used as trigger output.
- "OC3REF" : OC3REF signal is used as trigger output.
- "OC4REF" : OC4REF signal is used as trigger output.
+ Additional modes (on TRGO2 only):
+ - "OC5REF" : OC5REF signal is used as trigger output.
+ - "OC6REF" : OC6REF signal is used as trigger output.
+ - "compare_pulse_OC4REF":
+ OC4REF rising or falling edges generate pulses.
+ - "compare_pulse_OC6REF":
+ OC6REF rising or falling edges generate pulses.
+ - "compare_pulse_OC4REF_r_or_OC6REF_r":
+ OC4REF or OC6REF rising edges generate pulses.
+ - "compare_pulse_OC4REF_r_or_OC6REF_f":
+ OC4REF rising or OC6REF falling edges generate pulses.
+ - "compare_pulse_OC5REF_r_or_OC6REF_r":
+ OC5REF or OC6REF rising edges generate pulses.
+ - "compare_pulse_OC5REF_r_or_OC6REF_f":
+ OC5REF rising or OC6REF falling edges generate pulses.
What: /sys/bus/iio/devices/triggerX/master_mode
KernelVersion: 4.11
diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
index 0f1a2cf..a0031b7 100644
--- a/drivers/iio/trigger/stm32-timer-trigger.c
+++ b/drivers/iio/trigger/stm32-timer-trigger.c
@@ -14,19 +14,19 @@
#include <linux/module.h>
#include <linux/platform_device.h>
-#define MAX_TRIGGERS 6
+#define MAX_TRIGGERS 7
#define MAX_VALIDS 5
/* List the triggers created by each timer */
static const void *triggers_table[][MAX_TRIGGERS] = {
- { TIM1_TRGO, TIM1_CH1, TIM1_CH2, TIM1_CH3, TIM1_CH4,},
+ { TIM1_TRGO, TIM1_TRGO2, TIM1_CH1, TIM1_CH2, TIM1_CH3, TIM1_CH4,},
{ TIM2_TRGO, TIM2_CH1, TIM2_CH2, TIM2_CH3, TIM2_CH4,},
{ TIM3_TRGO, TIM3_CH1, TIM3_CH2, TIM3_CH3, TIM3_CH4,},
{ TIM4_TRGO, TIM4_CH1, TIM4_CH2, TIM4_CH3, TIM4_CH4,},
{ TIM5_TRGO, TIM5_CH1, TIM5_CH2, TIM5_CH3, TIM5_CH4,},
{ TIM6_TRGO,},
{ TIM7_TRGO,},
- { TIM8_TRGO, TIM8_CH1, TIM8_CH2, TIM8_CH3, TIM8_CH4,},
+ { TIM8_TRGO, TIM8_TRGO2, TIM8_CH1, TIM8_CH2, TIM8_CH3, TIM8_CH4,},
{ TIM9_TRGO, TIM9_CH1, TIM9_CH2,},
{ }, /* timer 10 */
{ }, /* timer 11 */
@@ -56,9 +56,16 @@ struct stm32_timer_trigger {
u32 max_arr;
const void *triggers;
const void *valids;
+ bool has_trgo2;
};
+static bool stm32_timer_is_trgo2_name(const char *name)
+{
+ return !!strstr(name, "trgo2");
+}
+
static int stm32_timer_start(struct stm32_timer_trigger *priv,
+ struct iio_trigger *trig,
unsigned int frequency)
{
unsigned long long prd, div;
@@ -102,7 +109,12 @@ static int stm32_timer_start(struct stm32_timer_trigger *priv,
regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_ARPE, TIM_CR1_ARPE);
/* Force master mode to update mode */
- regmap_update_bits(priv->regmap, TIM_CR2, TIM_CR2_MMS, 0x20);
+ if (stm32_timer_is_trgo2_name(trig->name))
+ regmap_update_bits(priv->regmap, TIM_CR2, TIM_CR2_MMS2,
+ 0x2 << TIM_CR2_MMS2_SHIFT);
+ else
+ regmap_update_bits(priv->regmap, TIM_CR2, TIM_CR2_MMS,
+ 0x2 << TIM_CR2_MMS_SHIFT);
/* Make sure that registers are updated */
regmap_update_bits(priv->regmap, TIM_EGR, TIM_EGR_UG, TIM_EGR_UG);
@@ -150,7 +162,7 @@ static ssize_t stm32_tt_store_frequency(struct device *dev,
if (freq == 0) {
stm32_timer_stop(priv);
} else {
- ret = stm32_timer_start(priv, freq);
+ ret = stm32_timer_start(priv, trig, freq);
if (ret)
return ret;
}
@@ -183,6 +195,9 @@ static IIO_DEV_ATTR_SAMP_FREQ(0660,
stm32_tt_read_frequency,
stm32_tt_store_frequency);
+#define MASTER_MODE_MAX 7
+#define MASTER_MODE2_MAX 15
+
static char *master_mode_table[] = {
"reset",
"enable",
@@ -191,7 +206,16 @@ static IIO_DEV_ATTR_SAMP_FREQ(0660,
"OC1REF",
"OC2REF",
"OC3REF",
- "OC4REF"
+ "OC4REF",
+ /* Master mode selection 2 only */
+ "OC5REF",
+ "OC6REF",
+ "compare_pulse_OC4REF",
+ "compare_pulse_OC6REF",
+ "compare_pulse_OC4REF_r_or_OC6REF_r",
+ "compare_pulse_OC4REF_r_or_OC6REF_f",
+ "compare_pulse_OC5REF_r_or_OC6REF_r",
+ "compare_pulse_OC5REF_r_or_OC6REF_f",
};
static ssize_t stm32_tt_show_master_mode(struct device *dev,
@@ -199,10 +223,15 @@ static ssize_t stm32_tt_show_master_mode(struct device *dev,
char *buf)
{
struct stm32_timer_trigger *priv = dev_get_drvdata(dev);
+ struct iio_trigger *trig = to_iio_trigger(dev);
u32 cr2;
regmap_read(priv->regmap, TIM_CR2, &cr2);
- cr2 = (cr2 & TIM_CR2_MMS) >> TIM_CR2_MMS_SHIFT;
+
+ if (stm32_timer_is_trgo2_name(trig->name))
+ cr2 = (cr2 & TIM_CR2_MMS2) >> TIM_CR2_MMS2_SHIFT;
+ else
+ cr2 = (cr2 & TIM_CR2_MMS) >> TIM_CR2_MMS_SHIFT;
return snprintf(buf, PAGE_SIZE, "%s\n", master_mode_table[cr2]);
}
@@ -212,13 +241,25 @@ static ssize_t stm32_tt_store_master_mode(struct device *dev,
const char *buf, size_t len)
{
struct stm32_timer_trigger *priv = dev_get_drvdata(dev);
+ struct iio_trigger *trig = to_iio_trigger(dev);
+ u32 mask, shift, master_mode_max;
int i;
- for (i = 0; i < ARRAY_SIZE(master_mode_table); i++) {
+ if (stm32_timer_is_trgo2_name(trig->name)) {
+ mask = TIM_CR2_MMS2;
+ shift = TIM_CR2_MMS2_SHIFT;
+ master_mode_max = MASTER_MODE2_MAX;
+ } else {
+ mask = TIM_CR2_MMS;
+ shift = TIM_CR2_MMS_SHIFT;
+ master_mode_max = MASTER_MODE_MAX;
+ }
+
+ for (i = 0; i <= master_mode_max; i++) {
if (!strncmp(master_mode_table[i], buf,
strlen(master_mode_table[i]))) {
- regmap_update_bits(priv->regmap, TIM_CR2,
- TIM_CR2_MMS, i << TIM_CR2_MMS_SHIFT);
+ regmap_update_bits(priv->regmap, TIM_CR2, mask,
+ i << shift);
/* Make sure that registers are updated */
regmap_update_bits(priv->regmap, TIM_EGR,
TIM_EGR_UG, TIM_EGR_UG);
@@ -229,8 +270,31 @@ static ssize_t stm32_tt_store_master_mode(struct device *dev,
return -EINVAL;
}
-static IIO_CONST_ATTR(master_mode_available,
- "reset enable update compare_pulse OC1REF OC2REF OC3REF OC4REF");
+static ssize_t stm32_tt_show_master_mode_avail(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct iio_trigger *trig = to_iio_trigger(dev);
+ unsigned int i, master_mode_max;
+ size_t len = 0;
+
+ if (stm32_timer_is_trgo2_name(trig->name))
+ master_mode_max = MASTER_MODE2_MAX;
+ else
+ master_mode_max = MASTER_MODE_MAX;
+
+ for (i = 0; i <= master_mode_max; i++)
+ len += scnprintf(buf + len, PAGE_SIZE - len,
+ "%s ", master_mode_table[i]);
+
+ /* replace trailing space by newline */
+ buf[len - 1] = '\n';
+
+ return len;
+}
+
+static IIO_DEVICE_ATTR(master_mode_available, 0444,
+ stm32_tt_show_master_mode_avail, NULL, 0);
static IIO_DEVICE_ATTR(master_mode, 0660,
stm32_tt_show_master_mode,
@@ -240,7 +304,7 @@ static IIO_DEVICE_ATTR(master_mode, 0660,
static struct attribute *stm32_trigger_attrs[] = {
&iio_dev_attr_sampling_frequency.dev_attr.attr,
&iio_dev_attr_master_mode.dev_attr.attr,
- &iio_const_attr_master_mode_available.dev_attr.attr,
+ &iio_dev_attr_master_mode_available.dev_attr.attr,
NULL,
};
@@ -264,6 +328,12 @@ static int stm32_setup_iio_triggers(struct stm32_timer_trigger *priv)
while (cur && *cur) {
struct iio_trigger *trig;
+ bool cur_is_trgo2 = stm32_timer_is_trgo2_name(*cur);
+
+ if (cur_is_trgo2 && !priv->has_trgo2) {
+ cur++;
+ continue;
+ }
trig = devm_iio_trigger_alloc(priv->dev, "%s", *cur);
if (!trig)
@@ -277,7 +347,7 @@ static int stm32_setup_iio_triggers(struct stm32_timer_trigger *priv)
* should only be available on trgo trigger which
* is always the first in the list.
*/
- if (cur == priv->triggers)
+ if (cur == priv->triggers || cur_is_trgo2)
trig->dev.groups = stm32_trigger_attr_groups;
iio_trigger_set_drvdata(trig, priv);
@@ -584,6 +654,20 @@ bool is_stm32_timer_trigger(struct iio_trigger *trig)
}
EXPORT_SYMBOL(is_stm32_timer_trigger);
+static void stm32_timer_detect_trgo2(struct stm32_timer_trigger *priv)
+{
+ u32 val;
+
+ /*
+ * Master mode selection 2 bits can only be written and read back when
+ * timer supports it.
+ */
+ regmap_update_bits(priv->regmap, TIM_CR2, TIM_CR2_MMS2, TIM_CR2_MMS2);
+ regmap_read(priv->regmap, TIM_CR2, &val);
+ regmap_update_bits(priv->regmap, TIM_CR2, TIM_CR2_MMS2, 0);
+ priv->has_trgo2 = !!val;
+}
+
static int stm32_timer_trigger_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -614,6 +698,7 @@ static int stm32_timer_trigger_probe(struct platform_device *pdev)
priv->max_arr = ddata->max_arr;
priv->triggers = triggers_table[index];
priv->valids = valids_table[index];
+ stm32_timer_detect_trgo2(priv);
ret = stm32_setup_iio_triggers(priv);
if (ret)
diff --git a/include/linux/iio/timer/stm32-timer-trigger.h b/include/linux/iio/timer/stm32-timer-trigger.h
index 55535ae..fa7d786 100644
--- a/include/linux/iio/timer/stm32-timer-trigger.h
+++ b/include/linux/iio/timer/stm32-timer-trigger.h
@@ -10,6 +10,7 @@
#define _STM32_TIMER_TRIGGER_H_
#define TIM1_TRGO "tim1_trgo"
+#define TIM1_TRGO2 "tim1_trgo2"
#define TIM1_CH1 "tim1_ch1"
#define TIM1_CH2 "tim1_ch2"
#define TIM1_CH3 "tim1_ch3"
@@ -44,6 +45,7 @@
#define TIM7_TRGO "tim7_trgo"
#define TIM8_TRGO "tim8_trgo"
+#define TIM8_TRGO2 "tim8_trgo2"
#define TIM8_CH1 "tim8_ch1"
#define TIM8_CH2 "tim8_ch2"
#define TIM8_CH3 "tim8_ch3"
diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
index 4a0abbc..ce7346e 100644
--- a/include/linux/mfd/stm32-timers.h
+++ b/include/linux/mfd/stm32-timers.h
@@ -34,6 +34,7 @@
#define TIM_CR1_DIR BIT(4) /* Counter Direction */
#define TIM_CR1_ARPE BIT(7) /* Auto-reload Preload Ena */
#define TIM_CR2_MMS (BIT(4) | BIT(5) | BIT(6)) /* Master mode selection */
+#define TIM_CR2_MMS2 GENMASK(23, 20) /* Master mode selection 2 */
#define TIM_SMCR_SMS (BIT(0) | BIT(1) | BIT(2)) /* Slave mode selection */
#define TIM_SMCR_TS (BIT(4) | BIT(5) | BIT(6)) /* Trigger selection */
#define TIM_DIER_UIE BIT(0) /* Update interrupt */
@@ -60,6 +61,7 @@
#define MAX_TIM_PSC 0xFFFF
#define TIM_CR2_MMS_SHIFT 4
+#define TIM_CR2_MMS2_SHIFT 20
#define TIM_SMCR_TS_SHIFT 4
#define TIM_BDTR_BKF_MASK 0xF
#define TIM_BDTR_BKF_SHIFT 16
--
1.9.1
^ permalink raw reply related
* [PATCH/RFT v3 renesas-devel] ARM: dts: silk: Enable UHS-I SDR-50
From: Simon Horman @ 2017-04-26 8:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170426080024.8391-1-horms+renesas@verge.net.au>
Add the "1v8" pinctrl state and sd-uhs-sdr50 property to SDHI1.
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
* Prepared on top of renesas-devel-20170424-v4.11-rc8
* Compile tested only; no access to silk board
v3
* Added missing pinctrl-1 to sdhi0
v2
* Correct mangled addition of sdhi*_pins
---
arch/arm/boot/dts/r8a7794-silk.dts | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
index 4cb5278d104d..8b416c385df7 100644
--- a/arch/arm/boot/dts/r8a7794-silk.dts
+++ b/arch/arm/boot/dts/r8a7794-silk.dts
@@ -196,6 +196,13 @@
sdhi1_pins: sd1 {
groups = "sdhi1_data4", "sdhi1_ctrl";
function = "sdhi1";
+ power-source = <3300>;
+ };
+
+ sdhi1_pins_uhs: sd1_uhs {
+ groups = "sdhi1_data4", "sdhi1_ctrl";
+ function = "sdhi1";
+ power-source = <1800>;
};
qspi_pins: qspi {
@@ -338,11 +345,13 @@
&sdhi1 {
pinctrl-0 = <&sdhi1_pins>;
- pinctrl-names = "default";
+ pinctrl-1 = <&sdhi0_pins_uhs>;
+ pinctrl-names = "default", "state_uhs";
vmmc-supply = <&vcc_sdhi1>;
vqmmc-supply = <&vccq_sdhi1>;
cd-gpios = <&gpio6 14 GPIO_ACTIVE_LOW>;
+ sd-uhs-sdr50;
status = "okay";
};
--
2.12.2.816.g2cccc81164
^ permalink raw reply related
* [PATCH v7 1/4] syscalls: Restore address limit after a syscall
From: Ingo Molnar @ 2017-04-26 8:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAJcbSZH7Y6iBy3T3iWyh_AaFJ+TidP7E9iQEuCbShQrcTKGoyw@mail.gmail.com>
* Thomas Garnier <thgarnie@google.com> wrote:
> >> +#ifdef CONFIG_ARCH_NO_SYSCALL_VERIFY_PRE_USERMODE_STATE
> >> +/*
> >> + * This function is called when an architecture specific implementation detected
> >> + * an invalid address limit. The generic user-mode state checker will finish on
> >> + * the appropriate BUG_ON.
> >> + */
> >> +asmlinkage void address_limit_check_failed(void)
> >> +{
> >> + verify_pre_usermode_state();
> >> + panic("address_limit_check_failed called with a valid user-mode state");
> >
> > It's very unconstructive to unconditionally panic the system, just because some
> > kernel code leaked the address limit! Do a warn-once printout and kill the current
> > task (i.e. don't continue execution), but don't crash everything else!
>
> The original change did not crash the kernel for this exact reason.
> Through reviews, there was an overall agreement that the kernel should
> not continue in this state.
Ok, I guess we can try that - but the panic message is still pretty misleading:
panic("address_limit_check_failed called with a valid user-mode state");
... so it was called with a _valid_ user-mode state, and we crash due to something
valid? Huh?
( Also, the style rule applies to kernel messages as well: function names should
be referred to as "function_name()". )
Thanks,
Ingo
^ permalink raw reply
* [PATCH/RFC 0/5] arm64: dts: renesas: Break out common board support
From: Geert Uytterhoeven @ 2017-04-26 8:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170426080642.GK25517@verge.net.au>
Hi Simon,
CC Vladimir (which I forgot to CC initially, sorry for that)
On Wed, Apr 26, 2017 at 10:06 AM, Simon Horman <horms@verge.net.au> wrote:
> On Fri, Apr 21, 2017 at 02:55:16PM +0200, Geert Uytterhoeven wrote:
>> The Renesas Salvator-X and ULCB development board can be equipped with
>> either an R-Car H3 or M3-W SiP, which are pin-compatible. All boards
>> use separate DTBs, but currently there's no sharing of board-specific
>> devices in DTS.
>>
>> This series reduces duplication by extracting common board support into
>> their own .dtsi files. As the level of support varies across boards and
>> SoCs, this requires the addition of a few external clocks and
>> placeholder devices on R-Car M3-W, so the common board support DTS can
>> refer to them.
>>
>> - Patches 1 and 2 add the external audio and PCIe bus clocks on R-Car
>> M3-W, which are present in r8a7795.dtsi, and used in
>> r8a7795-salvator-x.dts,
>> - RFC patch 3 adds placeholders for devices that are not yet supported
>> and/or tested on R-Car M3-W, but used on R-Car H3,
>> - RFC patch 4 extracts common Salvator-X board support,
>> - RFC patch 5 extracts common ULCB board support.
>>
>> For R-Car H3 based boards, there are no functional changes.
>> For R-Car M3-W based boards, some new devices are now described in DT.
>>
>> Dependencies:
>> - renesas-devel-20170420-v4.11-rc7,
>> - Patches 1 and 2 can be applied as-is,
>> - Patches 4 and 5 depend on "[PATCH 0/8] arm64: dts: renesas: Break
>> out R-Car H3 and M3-W SiP"
>> (http://www.spinics.net/lists/devicetree/msg173820.html).
>>
>> DTB changes have been inspected using scripts/dtc/dtx_diff.
>> This has been tested on Salvator-X (both H3 and M3-W).
>> This has not been tested on H3ULCB and M3ULCB due to lack of hardware.
>>
>> Thanks for your comments!
>
> Thanks for tackling this important problem. I have looked over the changes
> and they seem nice to me. I would, however, be more comfortable applying
> them if they were rested on the ULCB boards.
tested?
I've pushed a branch for testing to topic/rcar3-dtsi-sharing in
git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [SPAM]Re: [PATCH 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe
From: Ryder Lee @ 2017-04-26 8:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAK8P3a1vgi9VBZfRNq0dDKzmfe8FkHQtgz6fEXW9fyUnBHtaQQ@mail.gmail.com>
Hi
On Tue, 2017-04-25 at 14:18 +0200, Arnd Bergmann wrote:
> On Sun, Apr 23, 2017 at 10:19 AM, Ryder Lee <ryder.lee@mediatek.com> wrote:
> > Add documentation for PCIe host driver available in MT7623
> > series SoCs.
> >
> > Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
> > ---
> > .../bindings/pci/mediatek,mt7623-pcie.txt | 153 +++++++++++++++++++++
> > 1 file changed, 153 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/pci/mediatek,mt7623-pcie.txt
> >
> > diff --git a/Documentation/devicetree/bindings/pci/mediatek,mt7623-pcie.txt b/Documentation/devicetree/bindings/pci/mediatek,mt7623-pcie.txt
> > new file mode 100644
> > index 0000000..ee93ba2
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/mediatek,mt7623-pcie.txt
> > @@ -0,0 +1,153 @@
> > +Mediatek MT7623 PCIe controller
> > +
> > +Required properties:
> > +- compatible: Should contain "mediatek,mt7623-pcie".
>
> Did mediatek license the IP block from someone else or was it
> developed in-house? Is there a name and/or version identifier
> for the block itself other than identifying it as the one in mt7623?
Originally, it license from synopsys. Our designer add a wrapper to hide
the DBI detail so that we cannot use them directly. Perhaps I can call
it "mediatek,gen2v1-pcie", because we have a plan to upstream a in-house
Gen2 IP in the future.
> > +- device_type: Must be "pci"
> > +- reg: Base addresses and lengths of the pcie controller.
> > +- interrupts: A list of interrupt outputs of the controller.
>
> Please be more specific about what each interrupt is for, and how
> many there are.
OK.
> > +Required properties:
> > +- device_type: Must be "pci"
> > +- assigned-addresses: Address and size of the port configuration registers
> > +- reg: Only the first four bytes are used to refer to the correct bus number
> > + and device number.
> > +- #address-cells: Must be 3
> > +- #size-cells: Must be 2
> > +- ranges: Sub-ranges distributed from the PCIe controller node. An empty
> > + property is sufficient.
> > +- clocks: Must contain an entry for each entry in clock-names.
> > + See ../clocks/clock-bindings.txt for details.
> > +- clock-names: Must include the following entries:
> > + - sys_ck
> > +- resets: Must contain an entry for each entry in reset-names.
> > + See ../reset/reset.txt for details.
>
> This seems odd: you have a device that is simply identified as "pci"
> without any more specific ID, but you require additional properties
> (clocks, reset, ...) that are not part of the standard PCI binding.
>
> Can you clarify how the port devices related to the root device in
> this hardware design?
I will write clarify like this:
PCIe subsys includes one Host/PCI bridge and 3 PCIe MAC port. There
are 3 bus master for data access and 1 slave for configuration and
status register access. Each port has PIPE interface to PHY and
> Have you considered moving the nonstandard properties into the host
> bridge node and having that device deal with setting up the links
> to the other drivers? That way we could use the regular pcie
> port driver for the children.
>
OK, but I still want to use port->reset to catch reset properties in
driver.
> > +- reset-names: Must include the following entries:
> > + - pcie-reset
> > +- num-lanes: Number of lanes to use for this port.
> > +- phys: Must contain an entry for each entry in phy-names.
> > +- phy-names: Must include an entry for each sub node. Entries are of the form
> > + "pcie-phyN": where N ranges from 0 to the value specified for port number.
> > + See ../phy/phy-mt7623-pcie.txt for details.
>
> I think the name should not include the number of the port but rather
> be always the same here.
>
Hmm, I think it's better to keep the name here. It's more readable for
user to understand the relationship between port0 and phy0.
> Arnd
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* [PATCH 1/2] PCI: mediatek: Add Mediatek PCIe host controller support
From: Ryder Lee @ 2017-04-26 8:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAK8P3a39TFkJ5=jp8me+Dyg-Qi-N_1ZKxpJGvnhmPEo4csnFCw@mail.gmail.com>
Hi,
On Tue, 2017-04-25 at 14:38 +0200, Arnd Bergmann wrote:
> On Sun, Apr 23, 2017 at 10:19 AM, Ryder Lee <ryder.lee@mediatek.com> wrote:
>
> > +static inline bool mtk_pcie_link_is_up(struct mtk_pcie_port *port)
> > +{
> > + return !!(readl_relaxed(port->base + PCIE_LINK_STATUS) &
> > + PCIE_PORT_LINKUP);
> > +}
>
> If this is not performance critical, please use the regular readl() instead
> of readl_relaxed().
I will correct it.
> > +static bool mtk_pcie_valid_device(struct mtk_pcie *pcie,
> > + struct pci_bus *bus, int devfn)
> > +{
> > + struct mtk_pcie_port *port;
> > + struct pci_dev *dev;
> > + struct pci_bus *pbus;
> > +
> > + /* if there is no link, then there is no device */
> > + list_for_each_entry(port, &pcie->ports, list) {
> > + if (bus->number == 0 && port->index == PCI_SLOT(devfn) &&
> > + mtk_pcie_link_is_up(port)) {
> > + return true;
> > + } else if (bus->number != 0) {
> > + pbus = bus;
> > + do {
> > + dev = pbus->self;
> > + if (port->index == PCI_SLOT(dev->devfn) &&
> > + mtk_pcie_link_is_up(port)) {
> > + return true;
> > + }
> > + pbus = dev->bus;
> > + } while (dev->bus->number != 0);
> > + }
> > + }
> > +
> > + return false;
> > +}
>
>
>
>
> > +static int mtk_pcie_hw_rd_cfg(struct mtk_pcie *pcie, u32 bus, u32 devfn,
> > + int where, int size, u32 *val)
> > +{
> > + writel(PCIE_CONF_ADDR(where, PCI_FUNC(devfn), PCI_SLOT(devfn), bus),
> > + pcie->base + PCIE_CFG_ADDR);
> > +
> > + *val = 0;
> > +
> > + switch (size) {
> > + case 1:
> > + *val = readb(pcie->base + PCIE_CFG_DATA + (where & 3));
> > + break;
> > + case 2:
> > + *val = readw(pcie->base + PCIE_CFG_DATA + (where & 2));
> > + break;
> > + case 4:
> > + *val = readl(pcie->base + PCIE_CFG_DATA);
> > + break;
> > + }
> > +
> > + return PCIBIOS_SUCCESSFUL;
> > +}
>
> This is a fairly standard set of read/write operations. Can you change
> the pci_ops
> to use pci_generic_config_read/pci_generic_config_write and an appropriate
> map function instead?
OK I will add a .map_bus() like this:
{ .
writel(PCIE_CONF_ADDR(where, fun, slot, bus), base + PCIE_CFG_ADDR);
return base + PCIE_CFG_DATA + (where & 3);
}
> > +static int mtk_pcie_enable_ports(struct mtk_pcie *pcie)
> > +{
> > + struct device *dev = pcie->dev;
> > + struct mtk_pcie_port *port, *tmp;
> > + int err, linkup = 0;
> > +
> > + list_for_each_entry_safe(port, tmp, &pcie->ports, list) {
> > + err = clk_prepare_enable(port->sys_ck);
> > + if (err) {
> > + dev_err(dev, "failed to enable port%d clock\n",
> > + port->index);
> > + continue;
> > + }
> > +
> > + /* assert RC */
> > + reset_control_assert(port->reset);
> > + /* de-assert RC */
> > + reset_control_deassert(port->reset);
> > +
> > + /* power on PHY */
> > + err = phy_power_on(port->phy);
> > + if (err) {
> > + dev_err(dev, "failed to power on port%d phy\n",
> > + port->index);
> > + goto err_phy_on;
> > + }
> > +
> > + mtk_pcie_assert_ports(port);
> > +
>
> Similar to the comment I had for the binding, I wonder if it would be
> better to keep all the information about the ports in one place and
> then just deal with it at the root level.
>
> Alternatively, we could decide to standardize on the properties
> you have added to the pcie port node, but then I would handle
> them in the pcieport driver rather than in the host bridge driver.
Sorry, I'm not sure what you want me to do here.
I could move all clock operation in root level. But we need to keep the
reset and PHY operation sequence in the loop, In addition, we could
easily free resources if ports link fail.
How about moving this function to mtk_pcie_parse_and_add_res()?
> > +/*
> > + * This IP lacks interrupt status register to check or map INTx from
> > + * different devices at the same time.
> > + */
> > +static int __init mtk_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
> > +{
> > + struct mtk_pcie *pcie = dev->bus->sysdata;
> > + struct mtk_pcie_port *port;
> > +
> > + list_for_each_entry(port, &pcie->ports, list)
> > + if (port->index == slot)
> > + return port->irq;
> > +
> > + return -1;
> > +}
>
> This looks odd, what is it needed for specifically? It looks like
> it's broken for devices behind bridges, and the interrupt mapping
> should normally come from the interrupt-map property, without
> the need for a driver specific map_irq override.
Our hardware just has a GIC for each port and lacks interrupt status for
host driver to distinguish INTx. So I return port IRQ here.
> > +static int mtk_pcie_register_ports(struct mtk_pcie *pcie)
> > +{
> > + struct pci_bus *bus, *child;
> > +
> > + bus = pci_scan_root_bus(pcie->dev, 0, &mtk_pcie_ops, pcie,
> > + &pcie->resources);
>
> Can you use the new pci_register_host_bridge() method instead of
> pci_scan_root_bus() here?
May I know what's difference between pci_scan_root_bus() and using
pci_register_host_bridge() directly? What situation should we use it?
It seems that just tegra use this new method currently.
I'm not sure whether I can still use pci_scan_root_bus() here?
> ARnd
Thanks for the review!
^ permalink raw reply
* [PATCH/RFC 0/5] arm64: dts: renesas: Break out common board support
From: Simon Horman @ 2017-04-26 8:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1492779321-23939-1-git-send-email-geert+renesas@glider.be>
Hi Geert,
On Fri, Apr 21, 2017 at 02:55:16PM +0200, Geert Uytterhoeven wrote:
> Hi all,
>
> The Renesas Salvator-X and ULCB development board can be equipped with
> either an R-Car H3 or M3-W SiP, which are pin-compatible. All boards
> use separate DTBs, but currently there's no sharing of board-specific
> devices in DTS.
>
> This series reduces duplication by extracting common board support into
> their own .dtsi files. As the level of support varies across boards and
> SoCs, this requires the addition of a few external clocks and
> placeholder devices on R-Car M3-W, so the common board support DTS can
> refer to them.
>
> - Patches 1 and 2 add the external audio and PCIe bus clocks on R-Car
> M3-W, which are present in r8a7795.dtsi, and used in
> r8a7795-salvator-x.dts,
> - RFC patch 3 adds placeholders for devices that are not yet supported
> and/or tested on R-Car M3-W, but used on R-Car H3,
> - RFC patch 4 extracts common Salvator-X board support,
> - RFC patch 5 extracts common ULCB board support.
>
> For R-Car H3 based boards, there are no functional changes.
> For R-Car M3-W based boards, some new devices are now described in DT.
>
> Dependencies:
> - renesas-devel-20170420-v4.11-rc7,
> - Patches 1 and 2 can be applied as-is,
> - Patches 4 and 5 depend on "[PATCH 0/8] arm64: dts: renesas: Break
> out R-Car H3 and M3-W SiP"
> (http://www.spinics.net/lists/devicetree/msg173820.html).
>
> DTB changes have been inspected using scripts/dtc/dtx_diff.
> This has been tested on Salvator-X (both H3 and M3-W).
> This has not been tested on H3ULCB and M3ULCB due to lack of hardware.
>
> Thanks for your comments!
Thanks for tackling this important problem. I have looked over the changes
and they seem nice to me. I would, however, be more comfortable applying
them if they were rested on the ULCB boards.
^ permalink raw reply
* [PATCH] reset: sti: Use devm_kcalloc() in syscfg_reset_controller_register()
From: SF Markus Elfring @ 2017-04-26 8:05 UTC (permalink / raw)
To: linux-arm-kernel
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 26 Apr 2017 09:50:34 +0200
* A multiplication for the size determination of a memory allocation
indicated that an array data structure should be processed.
Thus use the corresponding function "devm_kcalloc".
* Replace the specification of a data structure by a pointer dereference
to make the corresponding size determination a bit safer according to
the Linux coding style convention.
* Delete the local variable "size" which became unnecessary with
this refactoring.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/reset/sti/reset-syscfg.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/reset/sti/reset-syscfg.c b/drivers/reset/sti/reset-syscfg.c
index 9bd57a5eee72..7e0f2aa55ba7 100644
--- a/drivers/reset/sti/reset-syscfg.c
+++ b/drivers/reset/sti/reset-syscfg.c
@@ -145,16 +145,14 @@ static int syscfg_reset_controller_register(struct device *dev,
const struct syscfg_reset_controller_data *data)
{
struct syscfg_reset_controller *rc;
- size_t size;
int i, err;
rc = devm_kzalloc(dev, sizeof(*rc), GFP_KERNEL);
if (!rc)
return -ENOMEM;
- size = sizeof(struct syscfg_reset_channel) * data->nr_channels;
-
- rc->channels = devm_kzalloc(dev, size, GFP_KERNEL);
+ rc->channels = devm_kcalloc(dev, data->nr_channels,
+ sizeof(*rc->channels), GFP_KERNEL);
if (!rc->channels)
return -ENOMEM;
--
2.12.2
^ permalink raw reply related
* [PATCH] irqchip/mbigen: Fix the clear register offset
From: Marc Zyngier @ 2017-04-26 8:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <68214147-bc4d-c542-ce40-78f69a63e53d@linaro.org>
On 26/04/17 04:10, Hanjun Guo wrote:
> Hi Majun,
>
> On 2017/4/25 10:16, Majun wrote:
>> From: MaJun <majun258@huawei.com>
>>
>> Don't minus reserved interrupts (64) when get the clear register offset,because
>> the clear register space includes the space of these 64 interrupts.
>
> Could you mention the background that there is a timeout mechanism
> to clear the register in the mbigen to make the code work even we clear
> the wrong (and noneffective) register? that will help for review I
> think.
A timeout? So if you don't clear the interrupt in a timely manner, it
will still bypass the masking? That feels very wrong. How is this
timeout configured? Can it be entirely disabled?
>
>>
>> Signed-off-by: MaJun <majun258@huawei.com>
>> ---
>> drivers/irqchip/irq-mbigen.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
>> index 061cdb8..75818a5 100644
>> --- a/drivers/irqchip/irq-mbigen.c
>> +++ b/drivers/irqchip/irq-mbigen.c
>> @@ -108,7 +108,6 @@ static inline void get_mbigen_clear_reg(irq_hw_number_t hwirq,
>> {
>> unsigned int ofst;
>>
>> - hwirq -= RESERVED_IRQ_PER_MBIGEN_CHIP;
>> ofst = hwirq / 32 * 4;
>>
>> *mask = 1 << (hwirq % 32);
>
> How about following to save more lines of code:
>
> --- a/drivers/irqchip/irq-mbigen.c
> +++ b/drivers/irqchip/irq-mbigen.c
> @@ -106,10 +106,7 @@ static inline void
> get_mbigen_type_reg(irq_hw_number_t hwirq,
> static inline void get_mbigen_clear_reg(irq_hw_number_t hwirq,
> u32 *mask, u32 *addr)
> {
> - unsigned int ofst;
> -
> - hwirq -= RESERVED_IRQ_PER_MBIGEN_CHIP;
> - ofst = hwirq / 32 * 4;
> + unsigned int ofst = hwirq / 32 * 4;
>
> *mask = 1 << (hwirq % 32);
> *addr = ofst + REG_MBIGEN_CLEAR_OFFSET;
Well, this is not a code deletion contest... ;-)
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH] ARM: dts: silk: Enable UHS-I SDR-50
From: Simon Horman @ 2017-04-26 8:00 UTC (permalink / raw)
To: linux-arm-kernel
Add the "1v8" pinctrl state and sd-uhs-sdr50 property to SDHI1.
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
* Compile tested only; no access to silk board
v3
* Added missing pinctrl-1 to sdhi0
v2
* Correct mangled addition of sdhi*_pins
---
arch/arm/boot/dts/r8a7794-silk.dts | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
index 4cb5278d104d..8b416c385df7 100644
--- a/arch/arm/boot/dts/r8a7794-silk.dts
+++ b/arch/arm/boot/dts/r8a7794-silk.dts
@@ -196,6 +196,13 @@
sdhi1_pins: sd1 {
groups = "sdhi1_data4", "sdhi1_ctrl";
function = "sdhi1";
+ power-source = <3300>;
+ };
+
+ sdhi1_pins_uhs: sd1_uhs {
+ groups = "sdhi1_data4", "sdhi1_ctrl";
+ function = "sdhi1";
+ power-source = <1800>;
};
qspi_pins: qspi {
@@ -338,11 +345,13 @@
&sdhi1 {
pinctrl-0 = <&sdhi1_pins>;
- pinctrl-names = "default";
+ pinctrl-1 = <&sdhi0_pins_uhs>;
+ pinctrl-names = "default", "state_uhs";
vmmc-supply = <&vcc_sdhi1>;
vqmmc-supply = <&vccq_sdhi1>;
cd-gpios = <&gpio6 14 GPIO_ACTIVE_LOW>;
+ sd-uhs-sdr50;
status = "okay";
};
--
2.12.2.816.g2cccc81164
^ permalink raw reply related
* [linux-sunxi] [PATCH 13/15] drm/sun4i: Add HDMI support
From: Chen-Yu Tsai @ 2017-04-26 7:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170426065005.zoewz53q7l7r5e7p@lukather>
On Wed, Apr 26, 2017 at 2:50 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi Chen-Yu,
>
> On Fri, Apr 21, 2017 at 11:17:17PM +0800, Chen-Yu Tsai wrote:
>> Hi,
>>
>> On Tue, Mar 7, 2017 at 4:56 PM, Maxime Ripard
>> <maxime.ripard@free-electrons.com> wrote:
>> > The earlier Allwinner SoCs (A10, A10s, A20, A31) have an embedded HDMI
>> > controller.
>> >
>> > That HDMI controller is able to do audio and CEC, but those have been left
>> > out for now.
>> >
>> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> > ---
>> > drivers/gpu/drm/sun4i/Makefile | 5 +-
>> > drivers/gpu/drm/sun4i/sun4i_hdmi.h | 124 ++++++-
>> > drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c | 128 ++++++-
>> > drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 449 +++++++++++++++++++++-
>> > drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c | 236 +++++++++++-
>> > 5 files changed, 942 insertions(+), 0 deletions(-)
>> > create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi.h
>> > create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c
>> > create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
>> > create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c
>>
>> Applying patch #9608371 using 'git am'
>> Description: [13/15] drm/sun4i: Add HDMI support
>> Applying: drm/sun4i: Add HDMI support
>> .git/rebase-apply/patch:116: trailing whitespace.
>>
>> .git/rebase-apply/patch:531: trailing whitespace.
>>
>> .git/rebase-apply/patch:701: trailing whitespace.
>>
>> warning: 3 lines add whitespace errors.
>
> Fixed.
>
>> > +int sun4i_ddc_create(struct sun4i_hdmi *hdmi, struct clk *parent)
>> > +{
>> > + struct clk_init_data init;
>> > + struct sun4i_ddc *ddc;
>> > + const char *parent_name;
>> > +
>> > + parent_name = __clk_get_name(parent);
>> > + if (!parent_name)
>> > + return -ENODEV;
>> > +
>> > + ddc = devm_kzalloc(hdmi->dev, sizeof(*ddc), GFP_KERNEL);
>> > + if (!ddc)
>> > + return -ENOMEM;
>> > +
>> > + init.name = "hdmi-ddc";
>> > + init.ops = &sun4i_ddc_ops;
>> > + init.parent_names = &parent_name;
>> > + init.num_parents = 1;
>> > + init.flags = CLK_SET_RATE_PARENT;
>>
>> I don't think this is really needed. It probably doesn't hurt though,
>> since DDC is used when HDMI is not used for displaying, but it might
>> affect any upstream PLLs, which theoretically may affect other users
>> of said PLLs. The DDC clock is slow enough that we should be able to
>> generate a usable clock rate anyway.
>
> Good point, I removed it.
>
>> > + writel(SUN4I_HDMI_VID_TIMING_X(mode->hdisplay) |
>> > + SUN4I_HDMI_VID_TIMING_Y(mode->vdisplay),
>> > + hdmi->base + SUN4I_HDMI_VID_TIMING_ACT_REG);
>> > +
>> > + x = mode->htotal - mode->hsync_start;
>> > + y = mode->vtotal - mode->vsync_start;
>>
>> I'm a bit skeptical about this one. All the other parameters are not
>> inclusive of other, why would this one be different? Shouldn't it
>> be "Xtotal - Xsync_end" instead?
>
> By the usual meaning of backporch, you're right. However, Allwinner's
> seems to have it's own, which is actually the backporch + sync length.
>
> We also have that on all the other connectors (and TCON), and this was
> confirmed at the time using a scope on an RGB signal.
Yes. On the later SoCs such as the A31, the user manual actually has
timing diagrams showing this.
Unlike the TCON, the HDMI controller's timings lists the front porch
separately, instead of an all inclusive Xtotal. This is what made me
look twice. This should be easy to confirm though. Since the HDMI modes
are well known and can be exactly reproduced on our hardware, we can
just check for any distortions or refresh rate errors.
>
>>
>> > + writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y),
>> > + hdmi->base + SUN4I_HDMI_VID_TIMING_BP_REG);
>> > +
>> > + x = mode->hsync_start - mode->hdisplay;
>> > + y = mode->vsync_start - mode->vdisplay;
>> > + writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y),
>> > + hdmi->base + SUN4I_HDMI_VID_TIMING_FP_REG);
>> > +
>> > + x = mode->hsync_end - mode->hsync_start;
>> > + y = mode->vsync_end - mode->vsync_start;
>> > + writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y),
>> > + hdmi->base + SUN4I_HDMI_VID_TIMING_SPW_REG);
>> > +
>> > + val = SUN4I_HDMI_VID_TIMING_POL_TX_CLK;
>> > + if (mode->flags & DRM_MODE_FLAG_PHSYNC)
>> > + val |= SUN4I_HDMI_VID_TIMING_POL_HSYNC;
>> > +
>> > + if (mode->flags & DRM_MODE_FLAG_PVSYNC)
>> > + val |= SUN4I_HDMI_VID_TIMING_POL_VSYNC;
>> > +
>> > + writel(val, hdmi->base + SUN4I_HDMI_VID_TIMING_POL_REG);
>>
>> You don't handle the interlaced video here, even though you set
>>
>> hdmi->connector.interlace_allowed = true
>>
>> later.
>
> I'll fix that.
>
>> The double clock and double scan flags aren't handled either, though
>> I don't understand which one is supposed to represent the need for the
>> HDMI pixel repeater. AFAIK this is required for resolutions with pixel
>> clocks lower than 25 MHz, the lower limit of HDMI's TMDS link.
>
> I'm not sure about this one though. I'd like to keep things quite
> simple for now and build up on that once the basis is working. Is it
> common in the wild?
If you drive the display at SDTV resolutions, then yes. Mode lines from
my HDMI monitor:
720x576i 50 720 732 795 864 576 580 586 625 flags: nhsync, nvsync,
interlace, dblclk; type: driver
720x480i 60 720 739 801 858 480 488 494 525 flags: nhsync, nvsync,
interlace, dblclk; type: driver
720x480i 60 720 739 801 858 480 488 494 525 flags: nhsync, nvsync,
interlace, dblclk; type: driver
AFAIK these are standard modes that all devices should support. Whether
they are used daily is another thing. Maybe block modes with dblclk
in .mode_fixup for now?
>> > + hdmi->base + SUN4I_HDMI_DDC_FIFO_CTRL_REG);
>> > + writel(SUN4I_HDMI_DDC_ADDR_SEGMENT(offset >> 8) |
>> > + SUN4I_HDMI_DDC_ADDR_EDDC(0x60) |
>> > + SUN4I_HDMI_DDC_ADDR_OFFSET(offset) |
>> > + SUN4I_HDMI_DDC_ADDR_SLAVE(0x50),
>>
>> You can use DDC_ADDR from drm_edid.h.
>
> Done.
There's also DDC_SEGMENT_ADDR (which is 0x30) you can use to replace 0x60.
The 1 bit shift is probably something related to I2C.
>> > +static enum drm_connector_status
>> > +sun4i_hdmi_connector_detect(struct drm_connector *connector, bool force)
>> > +{
>> > + struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector);
>> > + unsigned long reg;
>> > +
>> > + if (readl_poll_timeout(hdmi->base + SUN4I_HDMI_HPD_REG, reg,
>> > + reg & SUN4I_HDMI_HPD_HIGH,
>> > + 0, 500000))
>>
>> We shouldn't need to do polling here. It should just return the status
>> at the instance it's called. Instead we should have a worker that does
>> polling to check if something is plugged or unplugged. I don't see any
>> interrupt bits for this though. :(
>
> As far as I know, polling in detect is okay. Why would you want to
> remove it?
Hmm, I guess it only serves to debounce the detection, i.e. extend the
time period of validity from the instance the function is run to the
instance plus 500 ms.
To be clear I'm not against it. However this only really works when
the DRM subsystem is brought up. We still need something else for
hotplugging, which is what I was arguing for.
Regards
ChenYu
>> > + ret = drm_encoder_init(drm,
>> > + &hdmi->encoder,
>> > + &sun4i_hdmi_funcs,
>> > + DRM_MODE_ENCODER_TMDS,
>> > + NULL);
>> > + if (ret) {
>> > + dev_err(dev, "Couldn't initialise the HDMI encoder\n");
>> > + return ret;
>> > + }
>> > +
>> > + hdmi->encoder.possible_crtcs = BIT(0);
>>
>> You can use drm_of_find_possible_crtcs() now. See the TV encoder driver.
>
> Ack.
>
>> > +
>> > + drm_connector_helper_add(&hdmi->connector,
>> > + &sun4i_hdmi_connector_helper_funcs);
>> > + ret = drm_connector_init(drm, &hdmi->connector,
>> > + &sun4i_hdmi_connector_funcs,
>> > + DRM_MODE_CONNECTOR_HDMIA);
>> > + if (ret) {
>> > + dev_err(dev,
>> > + "Couldn't initialise the Composite connector\n");
>>
>> Wrong connector.
>
> Fixed.
>
>> > + ret = sun4i_ddc_create(hdmi, hdmi->tmds_clk);
>> > + if (ret) {
>> > + dev_err(&pdev->dev, "Couldn't create the DDC clock\n");
>> > + return ret;
>> > + }
>>
>> We do all this in the bind function for all the other components.
>> Any particular reason to do it differently here?
>
> Not really, I'll change it.
>
> Thanks!
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
^ permalink raw reply
* [PATCH/RFT v2] ARM: dts: silk: Enable UHS-I SDR-50
From: Simon Horman @ 2017-04-26 7:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170419072709.5kerc7y4437to2d3@ninjato>
On Wed, Apr 19, 2017 at 09:27:09AM +0200, Wolfram Sang wrote:
> On Tue, Apr 18, 2017 at 04:34:47PM +0900, Simon Horman wrote:
> > Add the "1v8" pinctrl state and sd-uhs-sdr50 property to SDHI1.
> >
> > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> > ---
> > * Compile tested only; no access to silk board
> > ---
> > arch/arm/boot/dts/r8a7794-silk.dts | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
> > index 4cb5278d104d..bb0c41c2f3ac 100644
> > --- a/arch/arm/boot/dts/r8a7794-silk.dts
> > +++ b/arch/arm/boot/dts/r8a7794-silk.dts
> > @@ -196,6 +196,13 @@
> > sdhi1_pins: sd1 {
> > groups = "sdhi1_data4", "sdhi1_ctrl";
> > function = "sdhi1";
> > + power-source = <3300>;
> > + };
> > +
> > + sdhi1_pins_uhs: sd1_uhs {
> > + groups = "sdhi1_data4", "sdhi1_ctrl";
> > + function = "sdhi1";
> > + power-source = <1800>;
> > };
> >
> > qspi_pins: qspi {
> > @@ -343,6 +350,7 @@
>
> No "pinctrl-1" for the uhs state?
My bad, I will send v3.
^ permalink raw reply
* [PATCH] arm64: dts: r8a7795: update PFC node name to pin-controller
From: Simon Horman @ 2017-04-26 7:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAMuHMdVw9OJZarw3SRa7NuuaMtmDKPVN3Om0GFVFm5k_YAqn6g@mail.gmail.com>
On Mon, Apr 24, 2017 at 11:03:54AM +0200, Geert Uytterhoeven wrote:
> On Mon, Apr 24, 2017 at 10:51 AM, Simon Horman
> <horms+renesas@verge.net.au> wrote:
> > The device trees for Renesas SoCs use either pfc or pin-controller
> > as the node name for the PFC device. This patch is intended to take a step
> > towards unifying the node name used as pin-controller which appears to
> > be more the more generic of the two and thus more in keeping with the DT
>
> s/be more/be/
>
> > specs.
> >
> > My analysis is that this is a user-visible change to the extent that kernel
> > logs, and sysfs entries change from e6060000.pfc and pfc at e6060000 to
> > e6060000.pin-controller and pin-controller at e6060000.
> >
> > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
>
> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
Thanks, I fixed the typo when queuing this up.
^ permalink raw reply
* [PATCH/RFC 0/2] clk / soc: renesas: Rework Kconfig and Makefile logic
From: Simon Horman @ 2017-04-26 7:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1493137626-21404-1-git-send-email-geert+renesas@glider.be>
On Tue, Apr 25, 2017 at 06:27:04PM +0200, Geert Uytterhoeven wrote:
> Hi all,
>
> This RFC patch series reworks the Kconfig and Makefile logic for the
> Renesas clock and SoC drivers. It was sparked by a discussion with Olof
> about the Renesas clock driver dependencies.
>
> The goals are to:
> - Allow precise control over and automatic selection of which
> (sub)drivers are used for which SoC (which may change in the
> future),
> - Allow adding support for new SoCs easily,
> - Allow compile-testing of all (sub)drivers,
> - Keep driver selection logic in the subsystem-specific Kconfig
> independent from the architecture-specific Kconfig (i.e. no "select"
> from arch/arm64/Kconfig.platforms), to avoid dependencies.
>
> The series can't be applied as-is, because compile-testing all drivers
> depends on independent fixes, some in other subsystems.
>
> More details are provided in the individual patch descriptions.
>
> For testing, this patch series, plus several fixes to allow more
> compile-testing, are available in the
> topic/clk-soc-renesas-Kconfig-rework branch of my renesas-drivers git
> repository at
> git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git.
>
> This has been tested on R-Car Gen2 and Gen3, and compile-tested for
> various other configurations.
>
> Thanks for your comments!
Hi Geert,
thanks for working on this. I am very pleased to see something that
is more robust with regards to adding new platforms without causing
build breakage - I have been burnt by this several times.
Acked-by: Simon Horman <horms+renesas@verge.net.au>
^ permalink raw reply
* Duplicate .plt sections warning with CONFIG_ARM_MODULE_PLTS
From: Ard Biesheuvel @ 2017-04-26 7:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1fb01fb6-3306-1d96-1da0-5b732555dfaf@gmail.com>
On 26 April 2017 at 00:35, Florian Fainelli <f.fainelli@gmail.com> wrote:
> Hi Ard,
>
> While using CONFIG_ARM_MODULES_PLTS=y along with a large kernel module,
> I was able to have the kernel/module.c::add_sect_attrs() to complain
> about duplicate .plt sections found, each section gets its own sysfs
> attribute created under /sys/module/<modname>/sections/<sectname>
>
> And indeed the module has the following section headers and contents
> (see below). Is this something that looks legit or should we patch
> kernel/module.c to detect such duplicates and generate unique sysfs
> attribute section names instead? There does not appear to be any
> functional issue with that.
>
Does it happen with a clean build? The .plt/.init.plt sections are
declared in the linker scripts, and so no module object should have
any such sections until the relocatable link occurs that combines them
all into a .ko, and it is at this point that these sections are
declared.
> Thanks!
>
> Contents of section .plt:
> 0000 00 .
> Contents of section .init.plt:
> 0001 00 .
> Contents of section .plt:
> 0000 00
>
> Section Headers:
> [Nr] Name Type Addr Off Size ES Flg
> Lk Inf Al
> [ 0] NULL 00000000 000000 000000 00
> 0 0 0
> [ 1] .text PROGBITS 00000000 000034 48d360 00 AX
> 0 0 4
> [ 2] .rel.text REL 00000000 68d0c4 14a980 08
> 48 1 4
> [ 3] .text.unlikely PROGBITS 00000000 48d394 001a30 00 AX
> 0 0 4
> [ 4] .rel.text.unlikel REL 00000000 7d7a44 000700 08
> 48 3 4
> [ 5] .rodata PROGBITS 00000000 48edc8 0b1fa4 00 A
> 0 0 8
> [ 6] .rel.rodata REL 00000000 7d8144 025938 08
> 48 5 4
> [ 7] .rodata.str1.1 PROGBITS 00000000 540d6c 10b88a 01 AMS
> 0 0 1
> [ 8] .ARM.extab.text.u PROGBITS 00000000 64c5f8 000060 00 A
> 0 0 4
> [ 9] .ARM.exidx.text.u ARM_EXIDX 00000000 64c658 000090 00 AL
> 3 0 4
> [10] .rel.ARM.exidx.te REL 00000000 7fda7c 000148 08
> 48 9 4
> [11] __ksymtab_strings PROGBITS 00000000 64c6e8 00da64 00 A
> 0 0 1
> [12] .modinfo PROGBITS 00000000 65a14c 0000a0 00 A
> 0 0 4
> [13] __param PROGBITS 00000000 65a1ec 000020 00 A
> 0 0 4
> [14] .rel__param REL 00000000 7fdbc4 000030 08
> 48 13 4
> [15] .alt.smp.init PROGBITS 00000000 65a20c 000028 00 A
> 0 0 4
> [16] .rel.alt.smp.init REL 00000000 7fdbf4 000028 08
> 48 15 4
> [17] __ex_table PROGBITS 00000000 65a238 000008 00 A
> 0 0 8
> [18] .rel__ex_table REL 00000000 7fdc1c 000010 08
> 48 17 4
> [19] .ARM.extab PROGBITS 00000000 65a240 002730 00 A
> 0 0 4
> [20] .ARM.exidx ARM_EXIDX 00000000 65c970 01b658 00 AL
> 1 0 4
> [21] .rel.ARM.exidx REL 00000000 7fdc2c 01f798 08
> 48 20 4
> [22] __versions PROGBITS 00000000 677fc8 001840 00 A
> 0 0 4
> [23] .note.gnu.build-i NOTE 00000000 679808 000048 00 A
> 0 0 4
> [24] .plt PROGBITS 00000000 679850 000001 00 WA
> 0 0 1
> [25] .init.plt PROGBITS 00000001 679851 000001 00 WA
> 0 0 1
> [26] .plt PROGBITS 00000000 679852 000001 00 WA
> 0 0 1
> [27] .data PROGBITS 00000000 679854 0039e8 00 WA
> 0 0 4
> [28] .rel.data REL 00000000 81d3c4 001ca0 08
> 48 27 4
> [29] .init.plt PROGBITS 00000000 67d23c 000001 00 WA
> 0 0 1
> [30] .gnu.linkonce.thi PROGBITS 00000000 67d240 000168 00 WA
> 0 0 4
> [31] .rel.gnu.linkonce REL 00000000 81f064 000010 08
> 48 30 4
> [32] .bss NOBITS 00000000 67d3a8 04dbac 00 WA
> 0 0 8
> [33] .comment PROGBITS 00000000 67d3a8 009106 01 MS
> 0 0 1
> [34] .note.GNU-stack PROGBITS 00000000 6864ae 000000 00 X
> 0 0 1
> [35] .ARM.attributes ARM_ATTRIBUTES 00000000 6864ae 000031 00
> 0 0 1
> [36] __ksymtab PROGBITS 00000000 6864e0 0036b8 00 A
> 0 0 4
> [37] .rel__ksymtab REL 00000000 81f074 006d70 08
> 48 36 4
> [38] __kcrctab PROGBITS 00000000 689b98 001b5c 00 A
> 0 0 4
> [39] .rel__kcrctab REL 00000000 825de4 0036b8 08
> 48 38 4
> [40] .debug_info PROGBITS 00000000 68b6f4 000490 00
> 0 0 1
> [41] .rel.debug_info REL 00000000 82949c 000408 08
> 48 40 4
> [42] .debug_abbrev PROGBITS 00000000 68bb84 0000f8 00
> 0 0 1
> [43] .debug_aranges PROGBITS 00000000 68bc7c 000018 00
> 0 0 1
> [44] .rel.debug_arange REL 00000000 8298a4 000008 08
> 48 43 4
> [45] .debug_line PROGBITS 00000000 68bc94 000190 00
> 0 0 1
> [46] .debug_str PROGBITS 00000000 68be24 000900 01 MS
> 0 0 1
> [47] .shstrtab STRTAB 00000000 68c724 0001cd 00
> 0 0 1
> [48] .symtab SYMTAB 00000000 8298ac 0ffc50 10
> 49 49732 4
> [49] .strtab STRTAB 00000000 9294fc 0f5791 00
> 0 0 1
>
> --
> Florian
^ permalink raw reply
* [RFC PATCH 30/30] vfio: Allow to bind foreign task
From: Tomasz Nowicki @ 2017-04-26 7:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170227195441.5170-31-jean-philippe.brucker@arm.com>
On 27.02.2017 20:54, Jean-Philippe Brucker wrote:
> Let the process that owns the device create an address space bond on
> behalf of another process. We add a pid argument to the BIND_TASK ioctl,
> allowing the caller to bind a foreign task. The expected program flow in
> this case is:
>
> * Process A creates the VFIO context and initializes the device.
> * Process B asks A to bind its address space.
> * Process A issues an ioctl to the VFIO device fd with BIND_TASK(pid).
> It may communicate the given PASID back to process B or keep track of it
> internally.
> * Process B asks A to perform transactions on its virtual address.
> * Process A launches transaction tagged with the given PASID.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
> ---
> drivers/vfio/vfio.c | 35 +++++++++++++++++++++++++++++++++--
> include/uapi/linux/vfio.h | 15 +++++++++++++++
> 2 files changed, 48 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index c4505d8f4c61..ecc5d07e3dbb 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -26,6 +26,7 @@
> #include <linux/module.h>
> #include <linux/mutex.h>
> #include <linux/pci.h>
> +#include <linux/ptrace.h>
> #include <linux/rwsem.h>
> #include <linux/sched.h>
> #include <linux/slab.h>
> @@ -1660,7 +1661,7 @@ static long vfio_svm_ioctl(struct vfio_device *device, unsigned int cmd,
> struct vfio_device_svm svm;
> struct vfio_task *vfio_task;
>
> - minsz = offsetofend(struct vfio_device_svm, pasid);
> + minsz = offsetofend(struct vfio_device_svm, pid);
>
> if (copy_from_user(&svm, (void __user *)arg, minsz))
> return -EFAULT;
> @@ -1669,9 +1670,39 @@ static long vfio_svm_ioctl(struct vfio_device *device, unsigned int cmd,
> return -EINVAL;
>
> if (cmd == VFIO_DEVICE_BIND_TASK) {
> - struct task_struct *task = current;
> + struct mm_struct *mm;
> + struct task_struct *task;
> +
> + if (svm.flags & ~VFIO_SVM_PID)
> + return -EINVAL;
> +
> + if (svm.flags & VFIO_SVM_PID) {
> + rcu_read_lock();
> + task = find_task_by_vpid(svm.pid);
> + if (task)
> + get_task_struct(task);
> + rcu_read_unlock();
> + if (!task)
> + return -ESRCH;
> +
> + /*
> + * Ensure process has RW access on the task's mm
> + * FIXME:
> + * - I think this ought to be in the IOMMU API
> + * - I'm assuming permission is never revoked during the
> + * task's lifetime. Might be mistaken.
> + */
> + mm = mm_access(task, PTRACE_MODE_ATTACH_REALCREDS);
> + if (!mm || IS_ERR(mm))
I know this is RFC patch but considering we will keep this as is, we
need here:
+put_task_struct(task);
> + return IS_ERR(mm) ? PTR_ERR(mm) : -ESRCH;
> + mmput(mm);
> + } else {
> + get_task_struct(current);
> + task = current;
> + }
>
> ret = iommu_bind_task(device->dev, task, &svm.pasid, 0, NULL);
> + put_task_struct(task);
> if (ret)
> return ret;
>
Thanks,
Tomasz
^ permalink raw reply
* [PATCH v2] arm64: perf: Use only exclude_kernel attribute when kernel is running in HYP
From: Pinski, Andrew @ 2017-04-26 7:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CA+7sy7D-ZB84xQ2pE-io1v7pmnAuWUszh1B=Zq+Ezup=sysQ2g@mail.gmail.com>
On 4/25/2017 11:53 PM, Jayachandran C. wrote:
> Hi Will,
>
> On Tue, Apr 25, 2017 at 10:23 PM, Will Deacon <will.deacon@arm.com> wrote:
>> On Tue, Apr 25, 2017 at 09:13:40AM +0530, Ganapatrao Kulkarni wrote:
>>> On Mon, Apr 24, 2017 at 9:15 PM, Will Deacon <will.deacon@arm.com> wrote:
>>>> On Thu, Apr 20, 2017 at 02:56:50PM +0530, Ganapatrao Kulkarni wrote:
>>>>> On Thu, Apr 20, 2017 at 2:19 PM, Mark Rutland <mark.rutland@arm.com> wrote:
>>>>>> On Wed, Apr 19, 2017 at 11:14:06PM +0530, Ganapatrao Kulkarni wrote:
>>>>>>> commit d98ecda (arm64: perf: Count EL2 events if the kernel is running in HYP)
>>>>>>> is returning error for perf syscall with mixed attribute set for exclude_kernel
>>>>>>> and exclude_hv. This change is breaking some applications (observed with hhvm)
>>>>>>> when ran on VHE enabled platforms.
>>>>>>>
>>>>>>> Adding fix to consider only exclude_kernel attribute when kernel is
>>>>>>> running in HYP. Also adding sysfs file to notify the bhehaviour
>>>>>>> of attribute exclude_hv.
>>>>>>>
>>>>>>> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
>>>>>>> ---
>>>>>>>
>>>>>>> Changelog:
>>>>>>>
>>>>>>> V2:
>>>>>>> - Changes as per Will Deacon's suggestion.
>>>>>>>
>>>>>>> V1: Initial patch
>>>>>>>
>>>>>>> arch/arm64/kernel/perf_event.c | 28 ++++++++++++++++++++++++----
>>>>>>> include/linux/perf/arm_pmu.h | 1 +
>>>>>>> 2 files changed, 25 insertions(+), 4 deletions(-)
>>>>>>>
>>>>>>> @@ -871,14 +890,13 @@ static int armv8pmu_set_event_filter(struct hw_perf_event *event,
>>>>>>>
>>>>>>> if (attr->exclude_idle)
>>>>>>> return -EPERM;
>>>>>>> - if (is_kernel_in_hyp_mode() &&
>>>>>>> - attr->exclude_kernel != attr->exclude_hv)
>>>>>>> - return -EINVAL;
>>>>>>> + if (is_kernel_in_hyp_mode() && !attr->exclude_kernel)
>>>>>>> + config_base |= ARMV8_PMU_INCLUDE_EL2;
>>>>>>> if (attr->exclude_user)
>>>>>>> config_base |= ARMV8_PMU_EXCLUDE_EL0;
>>>>>>> if (!is_kernel_in_hyp_mode() && attr->exclude_kernel)
>>>>>>> config_base |= ARMV8_PMU_EXCLUDE_EL1;
>>>>>>> - if (!attr->exclude_hv)
>>>>>>> + if (!is_kernel_in_hyp_mode() && !attr->exclude_hv)
>>>>>>> config_base |= ARMV8_PMU_INCLUDE_EL2;
>>>>>> This isn't quite what Will suggested.
>>>>>>
>>>>>> The idea was that userspace would read sysfs, then use that to determine
>>>>>> the correct exclusion parameters [1,2]. This logic was not expected to
>>>>>> change; it correctly validates whether we can provide what the user
>>>>>> requests.
>>>>> OK, if you are ok with sysfs part, i can send next version with that
>>>>> change only?.
>>>> I think the sysfs part is still a little dodgy, since you still expose the
>>>> "exclude_hv" file with a value of 0 when not running at EL2, which would
>>>> imply that exclude_hv is forced to zero. I don't think that's correct.
>>> okay, i can make exclude_hv visible only when kernel booted in EL2.
>>> is it ok to have empty directory "attr" when kernel booted to EL1?
>>> attr can be place holder for any other miscellaneous attributes, that
>>> can be added in future.
>> Sounds good to me, although I'll seek comment from the other perf folks
>> before merging anything with ABI implications.
> Do you really think this is the solution given:
> - this is an arm64 specific sysfs interface that is tied to the perf API
> - the perf API documentation has to be updated for this
> - All the applications that use the perf API have to be modified to
> check this sysfs interface
> - If the application fails to do so, a very narrow corner case
> (exclude_hv != exclude_kernel and VHE enabled) fails.
>
> Any application that really cares can already do see if exclude_hv !=
> exclude_kernel case works by calling perf_open_event() with those
> options and checking the return value.
An example of an application which needs to changed is HHVM. Currently
it sets exclude_hv to true but exclude_kernel to false as it does not
care about the hypervisor associated perf events associated with the
code, only the kernel and userspace associated evnts.
Yes we could submit a patch to use the sysfs interface to check but it
would look funny and the facebook folks might reject the patch as it is
ARM64 specific in generic code. Note this is how all of this discussion
started was HHVM's call to perf_open_event was failing.
Thanks,
Andrew Pinski
>
> Hope I am mistake here, otherwise this does not sound like a good idea.
>
> JC.
^ permalink raw reply
* [PATCH] arm64: dts: exynos: Remove the te-gpios property in the TM2 boards
From: Krzysztof Kozlowski @ 2017-04-26 7:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1493085261-3488-1-git-send-email-hoegeun.kwon@samsung.com>
On Tue, Apr 25, 2017 at 3:54 AM, Hoegeun Kwon <hoegeun.kwon@samsung.com> wrote:
> The decon uses HW-TRIGGER, so TE interrupt is not necessary.
> Therefore, remove the te-gpios property in the TM2 dts.
>
> Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
> ---
> arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 1 -
> 1 file changed, 1 deletion(-)
>
Looks correct. It is too late for me for v4.12 so I will pick it up
for v4.13. For my reference:
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Best regards,
Krzysztof
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox