With due respect
Dear Friend, I know that this mail will come to you as a surprise as we have never met before, but need not to worry as I am contacting you independently of my investigation and no one is informed of this communication. I need your urgent assistance in transferring the sum of $11.3million immediately to your private account.The money has been here in our Bank lying dormant for years now without anybody coming for the claim of it. I want to release the money to you as the relative to our deceased customer (the account owner) who died a long with his supposed NEXT OF KIN since 16th October 2005. The Banking laws here does not allow such money to stay more than 14 years, because the money will be recalled to the Bank treasury account as unclaimed fund. By indicating your interest I will send you the full details on how the business will be executed. Please respond urgently and delete if you are not interested. Best Regards, Mr. Duna Wattara.
[GIT PULL] pin control fixes for v5.0-rc
Hi Linus, here are some overdue pin control fixes for the v5.0 series. Was too busy to send them until now. Please pull them in! Yours, Linus Walleij The following changes since commit bfeffd155283772bbe78c6a05dec7c0128ee500c: Linux 5.0-rc1 (2019-01-06 17:08:20 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git tags/pinctrl-v5.0-2 for you to fetch changes up to 10098709b4ee6f6f19f25ba81d9c6f83518c584c: pinctrl: sunxi: Correct number of IRQ banks on H6 main pin controller (2019-01-22 10:52:39 +0100) Pin control fixes for the v5.0 series: - Mediatek Kconfig fix - Sunxi regulator, IRQ banks and pin base fixup - Intel Cherryview Strago DMI workaround - Potential regmap problem on mcp23s08 Chen-Yu Tsai (3): pinctrl: sunxi: Fix and simplify pin bank regulator handling pinctrl: sunxi: Consider pin_base when calculating regulator array index pinctrl: sunxi: Correct number of IRQ banks on H6 main pin controller Dmitry Torokhov (1): pinctrl: cherryview: fix Strago DMI workaround Jason Kridner (1): pinctrl: mcp23s08: spi: Fix regmap allocation for mcp23s18 Ryder Lee (1): pinctrl: mediatek: fix Kconfig build errors for moore core drivers/pinctrl/intel/pinctrl-cherryview.c | 8 +++--- drivers/pinctrl/mediatek/Kconfig | 3 ++ drivers/pinctrl/pinctrl-mcp23s08.c | 7 - drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c | 2 +- drivers/pinctrl/sunxi/pinctrl-sunxi.c | 44 +++--- drivers/pinctrl/sunxi/pinctrl-sunxi.h | 2 +- 6 files changed, 37 insertions(+), 29 deletions(-)
Re: [PATCH v2 1/7] pinctrl: at91: add option to use drive strength bits
On Thu, Jan 31, 2019 at 05:18:04PM +0100, Claudiu Beznea - M18063 wrote: > From: Claudiu Beznea > > SAM9X60 uses high and low drive strengths. To implement this, in > at91_pinctrl_mux_ops::set_drivestrength and > at91_pinctrl_mux_ops::get_drivestrength we need bit numbers of > drive strengths (1 for low, 2 for high), thus change the code to > allow the usage of drive strength bit numbers. > > Signed-off-by: Claudiu Beznea > --- > drivers/pinctrl/pinctrl-at91.c | 32 > 1 file changed, 20 insertions(+), 12 deletions(-) > > diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c > index 3d49bbbcdbc7..31f06dafca2e 100644 > --- a/drivers/pinctrl/pinctrl-at91.c > +++ b/drivers/pinctrl/pinctrl-at91.c > @@ -72,10 +72,15 @@ static int gpio_banks; > * DRIVE_STRENGTH_DEFAULT is just a placeholder to avoid changing the drive > * strength when there is no dt config for it. > */ > -#define DRIVE_STRENGTH_DEFAULT (0 << DRIVE_STRENGTH_SHIFT) > -#define DRIVE_STRENGTH_LOW (1 << DRIVE_STRENGTH_SHIFT) > -#define DRIVE_STRENGTH_MED (2 << DRIVE_STRENGTH_SHIFT) > -#define DRIVE_STRENGTH_HI (3 << DRIVE_STRENGTH_SHIFT) > +enum drive_strength_bit { > + DRIVE_STRENGTH_BIT_DEF, > + DRIVE_STRENGTH_BIT_LOW, > + DRIVE_STRENGTH_BIT_MED, > + DRIVE_STRENGTH_BIT_HI, > +}; > + > +#define DRIVE_STRENGTH_BIT_MSK(name) (DRIVE_STRENGTH_BIT_##name << \ > + DRIVE_STRENGTH_SHIFT) > > /** > * struct at91_pmx_func - describes AT91 pinmux functions > @@ -551,7 +556,7 @@ static unsigned at91_mux_sama5d3_get_drivestrength(void > __iomem *pio, > /* SAMA5 strength is 1:1 with our defines, >* except 0 is equivalent to low per datasheet */ > if (!tmp) > - tmp = DRIVE_STRENGTH_LOW; > + tmp = DRIVE_STRENGTH_BIT_MSK(LOW); > > return tmp; > } > @@ -564,7 +569,7 @@ static unsigned at91_mux_sam9x5_get_drivestrength(void > __iomem *pio, > > /* strength is inverse in SAM9x5s hardware with the pinctrl defines >* hardware: 0 = hi, 1 = med, 2 = low, 3 = rsvd */ > - tmp = DRIVE_STRENGTH_HI - tmp; > + tmp = DRIVE_STRENGTH_BIT_MSK(HI) - tmp; > > return tmp; > } > @@ -600,7 +605,7 @@ static void at91_mux_sam9x5_set_drivestrength(void > __iomem *pio, unsigned pin, > > /* strength is inverse on SAM9x5s with our defines >* 0 = hi, 1 = med, 2 = low, 3 = rsvd */ > - setting = DRIVE_STRENGTH_HI - setting; > + setting = DRIVE_STRENGTH_BIT_MSK(HI) - setting; > > set_drive_strength(pio + at91sam9x5_get_drive_register(pin), pin, > setting); > @@ -959,11 +964,11 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev, > } \ > } while (0) > > -#define DBG_SHOW_FLAG_MASKED(mask,flag) do { \ > +#define DBG_SHOW_FLAG_MASKED(mask,flag,name) do {\ checkpatch error: space required > if ((config & mask) == flag) { \ > if (num_conf) \ > seq_puts(s, "|"); \ > - seq_puts(s, #flag); \ > + seq_puts(s, #name); \ > num_conf++; \ > } \ > } while (0) > @@ -981,9 +986,12 @@ static void at91_pinconf_dbg_show(struct pinctrl_dev > *pctldev, > DBG_SHOW_FLAG(PULL_DOWN); > DBG_SHOW_FLAG(DIS_SCHMIT); > DBG_SHOW_FLAG(DEGLITCH); > - DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_LOW); > - DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_MED); > - DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_HI); > + DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_BIT_MSK(LOW), > + DRIVE_STRENGTH_LOW); > + DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_BIT_MSK(MED), > + DRIVE_STRENGTH_MED); > + DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_BIT_MSK(HI), > + DRIVE_STRENGTH_HI); > DBG_SHOW_FLAG(DEBOUNCE); > if (config & DEBOUNCE) { > val = config >> DEBOUNCE_VAL_SHIFT; > -- > 2.7.4 >
Re: [PATCH 1/9] pinctrl: sunxi: Support I/O bias voltage setting on A80
On Wed, Feb 6, 2019 at 4:32 AM Chen-Yu Tsai wrote: > The A80 SoC has configuration registers for I/O bias voltage. Incorrect > settings would make the affected peripherals inoperable in some cases, > such as Ethernet RGMII signals biased at 2.5V with the settings still > at 3.3V. However low speed signals such as MDIO on the same group of > pins seem to be unaffected. > > Previously there was no way to know what the actual voltage used was, > short of hard-coding a value in the device tree. With the new pin bank > regulator supply support in place, the driver can now query the > regulator for its voltage, and if it's valid (as opposed to being the > dummy regulator), set the bias voltage setting accordingly. > > Add a quirk to denote the presence of the configuration registers, and > a function to set the correct setting based on the voltage read back > from the regulator. > > This is only done when the regulator is first acquired and enabled. > While it would be nice to have a notifier on the regulator so that when > the voltage changes, the driver can update the setting, in practice no > board currently supports dynamic changing of the I/O voltages. > > Signed-off-by: Chen-Yu Tsai Hi Chen-Yu, thanks for the patch! I tried to apply it on the pinctrl devel branch (for v5.1) but it failed to apply, I assume because it depends on the fixes that I just sent to Torvalds. Shall we proceed like this that I merge v5.0-rc6 as soon as it is out and then try to apply this on top of that instead, so we get rid of this conflict? Yours, Linus Walleij
Re: [PATCH v8 1/6] pwm: extend PWM framework with PWM modes
Hello Thierry, On Wed, Feb 06, 2019 at 12:01:26AM +0100, Thierry Reding wrote: > On Mon, Jan 07, 2019 at 11:10:40PM +0100, Uwe Kleine-König wrote: > > On Mon, Jan 07, 2019 at 09:30:55AM +, claudiu.bez...@microchip.com > > wrote: > > > On 05.01.2019 23:05, Uwe Kleine-König wrote: > > > > On Thu, Jan 03, 2019 at 01:29:44PM +, claudiu.bez...@microchip.com > > > > wrote: > > > >> From: Claudiu Beznea > > > >> > > > >> Add basic PWM modes: normal and complementary. These modes should > > > >> differentiate the single output PWM channels from two outputs PWM > > > >> channels. These modes could be set as follow: > > > >> 1. PWM channels with one output per channel: > > > >> - normal mode > > > >> 2. PWM channels with two outputs per channel: > > > >> - normal mode > > > >> - complementary mode > > > >> Since users could use a PWM channel with two output as one output PWM > > > >> channel, the PWM normal mode is allowed to be set for PWM channels with > > > >> two outputs; in fact PWM normal mode should be supported by all PWMs. > > > > > > > > I still think that my suggestion that I sent in reply to your v5 using > > > > .alt_duty_cycle and .alt_offset is the better one as it is more generic. > > > > > > I like it better my way, I explained myself why. > > > > I couldn't really follow your argument though. You seemed to acknowledge > > that using .alt_duty_cycle and .alt_offset is more generic. Then you > > wrote that the push-pull mode is hardware generated on Atmel with some > > implementation details. IMHO these implementation details shouldn't be > > part of the PWM API and atmel's .apply should look as follows: > > > > if (state->alt_duty_cycle == 0) { > > > > ... configure for normal mode ... > > > > } else if (state->duty_cycle == state->alt_duty_cycle && > >state->alt_offset == state->period / 2) { > > > > ... configure for push pull mode ... > > > > } else if (state->duty_cycle + state->alt_duty_cycle == state->period && > >state->alt_offset == state->duty_cycle) { > > > > ... configure for complementary mode ... > > > > } else { > > return -EINVAL; > > } > > > > If it turns out to be a common pattern, we can add helper functions à la > > pwm_is_complementary_mode(state) and > > pwm_set_complementary_mode(state, period, duty_cycle). This allows to > > have a generic way to describe a wide range of wave forms in a uniform > > way in the API (which is good) and each driver implements the parts of > > this range that it can support. > > I think this is going to be the rule rather than the exception, so I'd > expect we'll see these helpers used in pretty much all drivers that > support more than just the normal mode. If you intended to contradict me here: You didn't. I have the same expectation. > But I really don't see the point in having consumers jump through hoops > to set one of the standard modes just to have the driver jump through > more hoops to determine which mode was meant. I think my approach is more natural and not more complicated at all. In all modes where this secondary output makes sense both outputs share the period length. In all modes both outputs have a falling and a raising edge each. Let's assume we support - normal mode (one output, secondary (if available) inactive) - complementary mode (secondary output is the inverse of primary output) - push-pull mode (primary output only does every second active phase, the secondy output does the ones that are skiped by the primary one) - complementary mode with deadtime (like above but there is a pause where both signals are inactive at the switch points, so the active phase of the secondary output is $deadtime_pre + $deadtime_post shorter than the primary output's inactive phase). To describe these modes we need with the approach suggested by Claudiu the following defines: enum mode { NORMAL, COMPLEMENTARY, PUSH_PULL PUSH_PULL_WITH_DEADTIME } struct pwm_state { unsigned int period; unsigned int duty_cycle; enum pwm_polarity polarity; bool enabled; enum mode mode; unsigned int deadtime_pre; unsigned int deadtime_post; } This has the following downsides: - The period in push-pull mode is somehow wrong because the signal combination repeats only every 2x $period ns. (I guess this is an implementation detail of the atmel hardware that leaks into the API here.) - There is redundancy in the description: { .period = U, .duty_cycle = V, .polarity = W, .enabled = true, .mode = PUSH_PULL, .deadtime_pre = DC, .deadtime_post = DC } is the same as { .period = U, .duty_cycle = V, .polarity = W, .enabled = true, .mode = PUSH_PULL_WITH_DEADTIME, .deadtime_pre = 0, .deadtime_post = 0 } . This is all more sane with my suggestion, and pwm_state is smaller with my approach. .period has always
Re: [PATCH] pinctrl: qcom: qcs404: Correct SDC tile
On Thu, Jan 31, 2019 at 6:30 PM Bjorn Andersson wrote: > The SDC controls live in the south tile, not the north one. Correct this > so that we program the right registers. > > Cc: sta...@vger.kernel.org > Fixes: 22eb8301dbc1 ("pinctrl: qcom: Add qcs404 pinctrl driver") > Signed-off-by: Bjorn Andersson Patch applied for fixes with Vinod's review tag. Yours, Linus Walleij
Re: [PATCH 01/35] ARM: davinci: remove intc_host_map from davinci_soc_info struct
Hi Bartosz, On 31/01/19 7:08 PM, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski > > This field is not used by any board. Remove it as part of the interrupt > support cleanup. Can you please make sure the patch description is independently readable without the subject line being read first. For this and some other patches in this series. > > Signed-off-by: Bartosz Golaszewski Thanks, Sekhar
Re: [v2] mtd: rawnand: gpmi: fix MX28 bus master lockup problem
From: Your Name On Tue, 2019-02-05 at 15:52:51 UTC, Martin Kepplinger wrote: > Disable BCH soft reset according to MX23 erratum #2847 ("BCH soft > reset may cause bus master lock up") for MX28 too. It has the same > problem. > > Observed problem: once per 100,000+ MX28 reboots NAND read failed on > DMA timeout errors: > [1.770823] UBI: attaching mtd3 to ubi0 > [2.768088] gpmi_nand: DMA timeout, last DMA :1 > [3.958087] gpmi_nand: BCH timeout, last DMA :1 > [4.156033] gpmi_nand: Error in ECC-based read: -110 > [4.161136] UBI warning: ubi_io_read: error -110 while reading 64 > bytes from PEB 0:0, read only 0 bytes, retry > [4.171283] step 1 error > [4.173846] gpmi_nand: Chip: 0, Error -1 > > Without BCH soft reset we successfully executed 1,000,000 MX28 reboots. > > I have a quote from NXP regarding this problem, from July 18th 2016: > > "As the i.MX23 and i.MX28 are of the same generation, they share many > characteristics. Unfortunately, also the erratas may be shared. > In case of the documented erratas and the workarounds, you can also > apply the workaround solution of one device on the other one. This have > been reported, but I’m afraid that there are not an estimated date for > updating the Errata documents. > Please accept our apologies for any inconveniences this may cause." > > Fixes: 6f2a6a52560a ("mtd: nand: gpmi: reset BCH earlier, too, to avoid > NAND startup problems") > Cc: sta...@vger.kernel.org > Signed-off-by: Manfred Schlaegl > Signed-off-by: Martin Kepplinger > Reviewed-by: Miquel Raynal > Reviewed-by: Fabio Estevam > Acked-by: Han Xu Applied to http://git.infradead.org/linux-mtd.git mtd/fixes, thanks. y'all come back now!
Re: [PATCH] bcache: use kmemdup_nul for CACHED_LABEL buffer
On 2019/1/30 5:29 下午, Geliang Tang wrote: > This patch uses kmemdup_nul to create a NUL-terminated string from > dc->sb.label. This is better than open coding it. > > With this, we can move env[2] initialization into env[] array to make > code more elegant. > > Signed-off-by: Geliang Tang Hi Geliang, In general I am OK with your idea. But I feel there might be some regression with your change. I comment your patch in line, correct me if I am wrong. > --- > drivers/md/bcache/super.c | 10 -- > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c > index 4dee119c3664..84ab241c8516 100644 > --- a/drivers/md/bcache/super.c > +++ b/drivers/md/bcache/super.c > @@ -906,21 +906,18 @@ static int cached_dev_status_update(void *arg) > void bch_cached_dev_run(struct cached_dev *dc) > { > struct bcache_device *d = &dc->disk; > - char buf[SB_LABEL_SIZE + 1]; > + char *buf = kmemdup_nul(dc->sb.label, SB_LABEL_SIZE, GFP_KERNEL); If kdumdup_null() is failed, buf will be NULL. > char *env[] = { > "DRIVER=bcache", > kasprintf(GFP_KERNEL, "CACHED_UUID=%pU", dc->sb.uuid), > - NULL, > + kasprintf(GFP_KERNEL, "CACHED_LABEL=%s", buf ? : ""), If buf is NULL, env[2] here is pointed to "" which is allocated in read-only data segment, and not a dynamic memory. > NULL, > }; > > - memcpy(buf, dc->sb.label, SB_LABEL_SIZE); > - buf[SB_LABEL_SIZE] = '\0'; > - env[2] = kasprintf(GFP_KERNEL, "CACHED_LABEL=%s", buf); > - > if (atomic_xchg(&dc->running, 1)) { > kfree(env[1]); > kfree(env[2]); Then kfree() here will try to release a read-only memory segment. I guess this is problematic. > + kfree(buf); > return; > } > > @@ -944,6 +941,7 @@ void bch_cached_dev_run(struct cached_dev *dc) > kobject_uevent_env(&disk_to_dev(d->disk)->kobj, KOBJ_CHANGE, env); > kfree(env[1]); > kfree(env[2]); Same problem might happen here for env[2]. > + kfree(buf); > > if (sysfs_create_link(&d->kobj, &disk_to_dev(d->disk)->kobj, "dev") || > sysfs_create_link(&disk_to_dev(d->disk)->kobj, &d->kobj, "bcache")) > -- Coly Li
Re: [PATCH] phy: Variable "val" in function miphy_osc_is_ready() could be uninitialized
Hi Yizhuo On 2/6/19 4:30 AM, Yizhuo wrote: > In function miphy_osc_is_ready(), local variable "val" > could be uninitalized. if function regmap_read() returns > -EINVAL. However, this value is used in if statement. > This is potentially unsafe. > > Signed-off-by: Yizhuo > --- > drivers/phy/st/phy-miphy28lp.c | 7 +-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/phy/st/phy-miphy28lp.c b/drivers/phy/st/phy-miphy28lp.c > index 213e2e15339c..40c7c0a611a0 100644 > --- a/drivers/phy/st/phy-miphy28lp.c > +++ b/drivers/phy/st/phy-miphy28lp.c > @@ -835,7 +835,8 @@ static int miphy_osc_is_ready(struct miphy28lp_phy > *miphy_phy) > { > struct miphy28lp_dev *miphy_dev = miphy_phy->phydev; > unsigned long finish = jiffies + 5 * HZ; > - u32 val; > + u32 val = 0; > + int ret; > > if (!miphy_phy->osc_rdy) > return 0; > @@ -844,8 +845,10 @@ static int miphy_osc_is_ready(struct miphy28lp_phy > *miphy_phy) > return -EINVAL; > > do { > - regmap_read(miphy_dev->regmap, > + ret = regmap_read(miphy_dev->regmap, > miphy_phy->syscfg_reg[SYSCFG_STATUS], &val); > + if (ret) > + return ret; > > if ((val & MIPHY_OSC_RDY) != MIPHY_OSC_RDY) > cpu_relax(); > Reviewed-by: Patrice Chotard Thanks
Re: [PATCH] usb: dwc3: Enable GBit Ethernet on Odroid XU4
Hi, On 21/01/19 16:02, Jochen Sprickerhof wrote: > Note that it only works with USB_XHCI_PLATFORM=y. Also it needs a hard > reset when coming from an unpatched kernel. > > This was included in the original patch in > https://patchwork.kernel.org/patch/9992809/ but got dropped when > accepted in d8c80bb3b55b phy: exynos5-usbdrd: Calibrate LOS levels for > exynos5420/5800. > > Old behaviour: > > $ lsusb -t > /: Bus 06.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/1p, 5000M > /: Bus 05.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/1p, 480M >|__ Port 1: Dev 2, If 0, Class=Vendor Specific Class, Driver=r8152, 480M > > New behaviour: > > $ lsusb -t > /: Bus 06.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/1p, 5000M >|__ Port 1: Dev 2, If 0, Class=Vendor Specific Class, Driver=r8152, 5000M > > Tested on Debian unstable using u-boot-exynos (2018.11+dfsg-2) and Linux > 4.19.14. > > Signed-off-by: Jochen Sprickerhof > --- > > Hi, > > I'm not sure why this it only works with the driver compiled into the > kernel nor why it needs a hard reset or why it was the line was dropped > when the patch was accepted. Would be great to get some feedback of the > authors. When XHCI driver is compiled into the kernel are the relevant PHY drivers compiled in as well? cheers, -roger > > Cheers Jochen > > drivers/usb/dwc3/core.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index a1b126f90261..0008bccc30aa 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -1169,7 +1169,8 @@ static int dwc3_core_init_mode(struct dwc3 *dwc) > dev_err(dev, "failed to initialize host\n"); > return ret; > } > -phy_calibrate(dwc->usb2_generic_phy); > +if (dwc->usb2_generic_phy) > +phy_calibrate(dwc->usb2_generic_phy); > break; > case USB_DR_MODE_OTG: > INIT_WORK(&dwc->drd_work, __dwc3_set_mode); -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Re: [PATCH v2] mtd: rawnand: gpmi: fix MX28 bus master lockup problem
Hi Martin, On Tue, 5 Feb 2019 16:52:51 +0100 Martin Kepplinger wrote: > Disable BCH soft reset according to MX23 erratum #2847 ("BCH soft > reset may cause bus master lock up") for MX28 too. It has the same > problem. > > Observed problem: once per 100,000+ MX28 reboots NAND read failed on > DMA timeout errors: > [1.770823] UBI: attaching mtd3 to ubi0 > [2.768088] gpmi_nand: DMA timeout, last DMA :1 > [3.958087] gpmi_nand: BCH timeout, last DMA :1 > [4.156033] gpmi_nand: Error in ECC-based read: -110 > [4.161136] UBI warning: ubi_io_read: error -110 while reading 64 > bytes from PEB 0:0, read only 0 bytes, retry > [4.171283] step 1 error > [4.173846] gpmi_nand: Chip: 0, Error -1 > > Without BCH soft reset we successfully executed 1,000,000 MX28 reboots. > > I have a quote from NXP regarding this problem, from July 18th 2016: > > "As the i.MX23 and i.MX28 are of the same generation, they share many > characteristics. Unfortunately, also the erratas may be shared. > In case of the documented erratas and the workarounds, you can also > apply the workaround solution of one device on the other one. This have > been reported, but I’m afraid that there are not an estimated date for > updating the Errata documents. > Please accept our apologies for any inconveniences this may cause." > > Fixes: 6f2a6a52560a ("mtd: nand: gpmi: reset BCH earlier, too, to avoid > NAND startup problems") Please make sure this Fixes line is not wrapped next time. Thanks, Boris
Re: [PATCH 1/1] RDMA/odp: convert to use HMM for ODP
On 1/29/2019 6:58 PM, jgli...@redhat.com wrote: > Convert ODP to use HMM so that we can build on common infrastructure > for different class of devices that want to mirror a process address > space into a device. There is no functional changes. Thanks for sending this patch. I think in general it is a good idea to use a common infrastructure for ODP. I have a couple of questions below. > -static void ib_umem_notifier_invalidate_range_end(struct mmu_notifier *mn, > - const struct mmu_notifier_range *range) > -{ > - struct ib_ucontext_per_mm *per_mm = > - container_of(mn, struct ib_ucontext_per_mm, mn); > - > - if (unlikely(!per_mm->active)) > - return; > - > - rbt_ib_umem_for_each_in_range(&per_mm->umem_tree, range->start, > - range->end, > - invalidate_range_end_trampoline, true, > NULL); > up_read(&per_mm->umem_rwsem); > + return ret; > } Previously the code held the umem_rwsem between range_start and range_end calls. I guess that was in order to guarantee that no device page faults take reference to the pages being invalidated while the invalidation is ongoing. I assume this is now handled by hmm instead, correct? > + > +static uint64_t odp_hmm_flags[HMM_PFN_FLAG_MAX] = { > + ODP_READ_BIT, /* HMM_PFN_VALID */ > + ODP_WRITE_BIT, /* HMM_PFN_WRITE */ > + ODP_DEVICE_BIT, /* HMM_PFN_DEVICE_PRIVATE */ It seems that the mlx5_ib code in this patch currently ignores the ODP_DEVICE_BIT (e.g., in umem_dma_to_mtt). Is that okay? Or is it handled implicitly by the HMM_PFN_SPECIAL case? > @@ -327,9 +287,10 @@ void put_per_mm(struct ib_umem_odp *umem_odp) > up_write(&per_mm->umem_rwsem); > > WARN_ON(!RB_EMPTY_ROOT(&per_mm->umem_tree.rb_root)); > - mmu_notifier_unregister_no_release(&per_mm->mn, per_mm->mm); > + hmm_mirror_unregister(&per_mm->mirror); > put_pid(per_mm->tgid); > - mmu_notifier_call_srcu(&per_mm->rcu, free_per_mm); > + > + kfree(per_mm); > } Previously the per_mm struct was released through call srcu, but now it is released immediately. Is it safe? I saw that hmm_mirror_unregister calls mmu_notifier_unregister_no_release, so I don't understand what prevents concurrently running invalidations from accessing the released per_mm struct. > @@ -578,11 +578,27 @@ static int pagefault_mr(struct mlx5_ib_dev *dev, struct > mlx5_ib_mr *mr, > > next_mr: > size = min_t(size_t, bcnt, ib_umem_end(&odp->umem) - io_virt); > - > page_shift = mr->umem->page_shift; > page_mask = ~(BIT(page_shift) - 1); > + off = (io_virt & (~page_mask)); > + size += (io_virt & (~page_mask)); > + io_virt = io_virt & page_mask; > + off += (size & (~page_mask)); > + size = ALIGN(size, 1UL << page_shift); > + > + if (io_virt < ib_umem_start(&odp->umem)) > + return -EINVAL; > + > start_idx = (io_virt - (mr->mmkey.iova & page_mask)) >> page_shift; > > + if (odp_mr->per_mm == NULL || odp_mr->per_mm->mm == NULL) > + return -ENOENT; > + > + ret = hmm_range_register(&range, odp_mr->per_mm->mm, > + io_virt, io_virt + size, page_shift); > + if (ret) > + return ret; > + > if (prefetch && !downgrade && !mr->umem->writable) { > /* prefetch with write-access must >* be supported by the MR Isn't there a mistake in the calculation of the variable size? Itis first set to the size of the page fault range, but then you add the virtual address, so I guess it is actually the range end. Then you pass io_virt + size to hmm_range_register. Doesn't it double the size of the range > -void ib_umem_odp_unmap_dma_pages(struct ib_umem_odp *umem_odp, u64 virt, > - u64 bound) > +void ib_umem_odp_unmap_dma_pages(struct ib_umem_odp *umem_odp, > + u64 virt, u64 bound) > { > + struct device *device = umem_odp->umem.context->device->dma_device; > struct ib_umem *umem = &umem_odp->umem; > - int idx; > - u64 addr; > - struct ib_device *dev = umem->context->device; > + unsigned long idx, page_mask; > + struct hmm_range range; > + long ret; > + > + if (!umem->npages) > + return; > + > + bound = ALIGN(bound, 1UL << umem->page_shift); > + page_mask = ~(BIT(umem->page_shift) - 1); > + virt &= page_mask; > > virt = max_t(u64, virt, ib_umem_start(umem)); > bound = min_t(u64, bound, ib_umem_end(umem)); > - /* Note that during the run of this function, the > - * notifiers_count of the MR is > 0, preventing any racing > - * faults from completion. We might be racing with other > - * invalidations, so we must make sure we free each page only > - * once. */ > + > + idx = ((unsigned long)virt - ib_umem_start(umem)) >> PAGE_SHIFT; > + > + range.p
Re: [PATCH 1/4] powerpc/64s: Clear on-stack exception marker upon exception return
On Wed, Feb 6, 2019 at 3:44 PM Michael Ellerman wrote: > > Balbir Singh writes: > > On Tue, Feb 5, 2019 at 10:24 PM Michael Ellerman > > wrote: > >> Balbir Singh writes: > >> > On Sat, Feb 2, 2019 at 12:14 PM Balbir Singh > >> > wrote: > >> >> On Tue, Jan 22, 2019 at 10:57:21AM -0500, Joe Lawrence wrote: > >> >> > From: Nicolai Stange > >> >> > > >> >> > The ppc64 specific implementation of the reliable stacktracer, > >> >> > save_stack_trace_tsk_reliable(), bails out and reports an "unreliable > >> >> > trace" whenever it finds an exception frame on the stack. Stack frames > >> >> > are classified as exception frames if the STACK_FRAME_REGS_MARKER > >> >> > magic, > >> >> > as written by exception prologues, is found at a particular location. > >> >> > > >> >> > However, as observed by Joe Lawrence, it is possible in practice that > >> >> > non-exception stack frames can alias with prior exception frames and > >> >> > thus, > >> >> > that the reliable stacktracer can find a stale > >> >> > STACK_FRAME_REGS_MARKER on > >> >> > the stack. It in turn falsely reports an unreliable stacktrace and > >> >> > blocks > >> >> > any live patching transition to finish. Said condition lasts until the > >> >> > stack frame is overwritten/initialized by function call or other > >> >> > means. > >> >> > > >> >> > In principle, we could mitigate this by making the exception frame > >> >> > classification condition in save_stack_trace_tsk_reliable() stronger: > >> >> > in addition to testing for STACK_FRAME_REGS_MARKER, we could also > >> >> > take into > >> >> > account that for all exceptions executing on the kernel stack > >> >> > - their stack frames's backlink pointers always match what is saved > >> >> > in their pt_regs instance's ->gpr[1] slot and that > >> >> > - their exception frame size equals STACK_INT_FRAME_SIZE, a value > >> >> > uncommonly large for non-exception frames. > >> >> > > >> >> > However, while these are currently true, relying on them would make > >> >> > the > >> >> > reliable stacktrace implementation more sensitive towards future > >> >> > changes in > >> >> > the exception entry code. Note that false negatives, i.e. not > >> >> > detecting > >> >> > exception frames, would silently break the live patching consistency > >> >> > model. > >> >> > > >> >> > Furthermore, certain other places (diagnostic stacktraces, perf, xmon) > >> >> > rely on STACK_FRAME_REGS_MARKER as well. > >> >> > > >> >> > Make the exception exit code clear the on-stack > >> >> > STACK_FRAME_REGS_MARKER > >> >> > for those exceptions running on the "normal" kernel stack and > >> >> > returning > >> >> > to kernelspace: because the topmost frame is ignored by the reliable > >> >> > stack > >> >> > tracer anyway, returns to userspace don't need to take care of > >> >> > clearing > >> >> > the marker. > >> >> > > >> >> > Furthermore, as I don't have the ability to test this on Book 3E or > >> >> > 32 bits, limit the change to Book 3S and 64 bits. > >> >> > > >> >> > Finally, make the HAVE_RELIABLE_STACKTRACE Kconfig option depend on > >> >> > PPC_BOOK3S_64 for documentation purposes. Before this patch, it > >> >> > depended > >> >> > on PPC64 && CPU_LITTLE_ENDIAN and because CPU_LITTLE_ENDIAN implies > >> >> > PPC_BOOK3S_64, there's no functional change here. > >> >> > > >> >> > Fixes: df78d3f61480 ("powerpc/livepatch: Implement reliable stack > >> >> > tracing for the consistency model") > >> >> > Reported-by: Joe Lawrence > >> >> > Signed-off-by: Nicolai Stange > >> >> > Signed-off-by: Joe Lawrence > >> >> > --- > >> >> > arch/powerpc/Kconfig | 2 +- > >> >> > arch/powerpc/kernel/entry_64.S | 7 +++ > >> >> > 2 files changed, 8 insertions(+), 1 deletion(-) > >> >> > > >> >> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > >> >> > index 2890d36eb531..73bf87b1d274 100644 > >> >> > --- a/arch/powerpc/Kconfig > >> >> > +++ b/arch/powerpc/Kconfig > >> >> > @@ -220,7 +220,7 @@ config PPC > >> >> > select HAVE_PERF_USER_STACK_DUMP > >> >> > select HAVE_RCU_TABLE_FREE if SMP > >> >> > select HAVE_REGS_AND_STACK_ACCESS_API > >> >> > - select HAVE_RELIABLE_STACKTRACE if PPC64 && > >> >> > CPU_LITTLE_ENDIAN > >> >> > + select HAVE_RELIABLE_STACKTRACE if PPC_BOOK3S_64 && > >> >> > CPU_LITTLE_ENDIAN > >> >> > select HAVE_SYSCALL_TRACEPOINTS > >> >> > select HAVE_VIRT_CPU_ACCOUNTING > >> >> > select HAVE_IRQ_TIME_ACCOUNTING > >> >> > diff --git a/arch/powerpc/kernel/entry_64.S > >> >> > b/arch/powerpc/kernel/entry_64.S > >> >> > index 435927f549c4..a2c168b395d2 100644 > >> >> > --- a/arch/powerpc/kernel/entry_64.S > >> >> > +++ b/arch/powerpc/kernel/entry_64.S > >> >> > @@ -1002,6 +1002,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR) > >> >> > ld r2,_NIP(r1) > >> >> > mtspr SPRN_SRR0,r2 > >> >> > > >> >> > + /* > >> >> > + * Leaving a stale exception_marker on the stack can confus
Re: [PATCH] USB: dwc3: add missing of_node_put()
Hi Wen On 2/3/19 4:52 AM, wen yang wrote: > The call to of_find_node_by_name returns a node pointer with refcount > incremented thus it must be explicitly decremented here after the last > usage. > The of_find_device_by_node() takes a reference to the underlying device > structure, we also should release that reference. > This patch fixes those 2 issues. > > Signed-off-by: Wen Yang > Cc: Patrice Chotard > Cc: Felipe Balbi > Cc: Greg Kroah-Hartman > Cc: linux-arm-ker...@lists.infradead.org > Cc: linux-...@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > --- > drivers/usb/dwc3/dwc3-st.c | 10 +++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c > index 1608138..fdd90d2 100644 > --- a/drivers/usb/dwc3/dwc3-st.c > +++ b/drivers/usb/dwc3/dwc3-st.c > @@ -262,17 +262,18 @@ static int st_dwc3_probe(struct platform_device *pdev) > ret = of_platform_populate(node, NULL, NULL, dev); > if (ret) { > dev_err(dev, "failed to add dwc3 core\n"); > - goto undo_softreset; > + goto put_node; > } > > child_pdev = of_find_device_by_node(child); > if (!child_pdev) { > dev_err(dev, "failed to find dwc3 core device\n"); > ret = -ENODEV; > - goto undo_softreset; > + goto put_node; > } > > dwc3_data->dr_mode = usb_get_dr_mode(&child_pdev->dev); > + put_device(&child_pdev->dev); > > /* >* Configure the USB port as device or host according to the static > @@ -283,15 +284,18 @@ static int st_dwc3_probe(struct platform_device *pdev) > ret = st_dwc3_drd_init(dwc3_data); > if (ret) { > dev_err(dev, "drd initialisation failed\n"); > - goto undo_softreset; > + goto put_node; > } > > /* ST glue logic init */ > st_dwc3_init(dwc3_data); > > platform_set_drvdata(pdev, dwc3_data); > + of_node_put(child); > return 0; > > +put_node: > + of_node_put(child); > undo_softreset: > reset_control_assert(dwc3_data->rstc_rst); > undo_powerdown: > Reviewed-by: Patrice Chotard Thanks
Re: [PATCH 2/4] pwm: stm32-lp: Add power management support
On 2/5/19 7:30 PM, Tomasz Duszynski wrote: > On Tue, Feb 05, 2019 at 01:40:27PM +0100, Fabrice Gasnier wrote: >> Add suspend/resume PM sleep ops. When going to low power, disable >> active PWM channel. Active PWM channel is resumed, by calling >> pwm_apply_state(). This is inspired by Thierry's comment in [1]. >> Don't touch inactive channels, as it may be used by other LPTimer MFD >> child driver. >> [1]https://lkml.org/lkml/2017/12/5/175 >> >> Signed-off-by: Fabrice Gasnier >> --- >> drivers/pwm/pwm-stm32-lp.c | 38 ++ >> 1 file changed, 38 insertions(+) >> >> diff --git a/drivers/pwm/pwm-stm32-lp.c b/drivers/pwm/pwm-stm32-lp.c >> index 0059b24c..0c40d48 100644 >> --- a/drivers/pwm/pwm-stm32-lp.c >> +++ b/drivers/pwm/pwm-stm32-lp.c >> @@ -13,6 +13,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> >> @@ -20,6 +21,8 @@ struct stm32_pwm_lp { >> struct pwm_chip chip; >> struct clk *clk; >> struct regmap *regmap; >> +struct pwm_state suspend; >> +bool suspended; >> }; >> >> static inline struct stm32_pwm_lp *to_stm32_pwm_lp(struct pwm_chip *chip) >> @@ -223,6 +226,40 @@ static int stm32_pwm_lp_remove(struct platform_device >> *pdev) >> return pwmchip_remove(&priv->chip); >> } >> >> +#ifdef CONFIG_PM_SLEEP > > You might consider dropping ifdefs and marking pm functions with > __maybe_unused instead. In case CONFIG_PM_SLEEP=n then these two guys > will be removed and pm ops structure will be empty. Hi Tomasz, Thanks for this suggestion. I can do this change. I'll wait for more feedback from Uwe and Thierry before sending a v2 with that. > >> +static int stm32_pwm_lp_suspend(struct device *dev) >> +{ >> +struct stm32_pwm_lp *priv = dev_get_drvdata(dev); >> + > > I guess you first need to get platform_device from dev and eventually > stm32_pwm_lp. Wondering how this works now. This should be safe for now. This works because the probe routine calls: platform_set_drvdata(pdev, priv); And the underlying call is dev_set_drvdata() static inline void platform_set_drvdata(struct platform_device *pdev, void *data) { dev_set_drvdata(&pdev->dev, data); } > >> +pwm_get_state(&priv->chip.pwms[0], &priv->suspend); >> +priv->suspended = priv->suspend.enabled; >> + >> +/* safe to call pwm_disable() for already disabled pwm */ >> +pwm_disable(&priv->chip.pwms[0]); >> + >> +return pinctrl_pm_select_sleep_state(dev); >> +} >> + >> +static int stm32_pwm_lp_resume(struct device *dev) >> +{ >> +struct stm32_pwm_lp *priv = dev_get_drvdata(dev); >> +int ret; >> + >> +ret = pinctrl_pm_select_default_state(dev); >> +if (ret) >> +return ret; >> + >> +/* Only restore suspended pwm, not to disrupt other MFD child */ >> +if (!priv->suspended) >> +return 0; > > Would it make sense to use suspend.enabled directly? I propose to keep priv->suspended. Using 'suspend.enabled' directly would simply not work as the pwm_disable() call in stm32_pwm_lp_suspend() routine marks the 'suspend' state.enabled = false. That's why it's saved in the suspend routine, to be restored upon resume. Thanks for reviewing, Best regards, Fabrice > >> + >> +return pwm_apply_state(&priv->chip.pwms[0], &priv->suspend); >> +} >> +#endif >> + >> +static SIMPLE_DEV_PM_OPS(stm32_pwm_lp_pm_ops, stm32_pwm_lp_suspend, >> + stm32_pwm_lp_resume); >> + >> static const struct of_device_id stm32_pwm_lp_of_match[] = { >> { .compatible = "st,stm32-pwm-lp", }, >> {}, >> @@ -235,6 +272,7 @@ static int stm32_pwm_lp_remove(struct platform_device >> *pdev) >> .driver = { >> .name = "stm32-pwm-lp", >> .of_match_table = of_match_ptr(stm32_pwm_lp_of_match), >> +.pm = &stm32_pwm_lp_pm_ops, >> }, >> }; >> module_platform_driver(stm32_pwm_lp_driver); >> -- >> 1.9.1 >>
Re: [PATCH 2/4] pwm: stm32-lp: Add power management support
On 2/5/19 11:25 PM, Thierry Reding wrote: > On Tue, Feb 05, 2019 at 09:47:32PM +0100, Uwe Kleine-König wrote: >> Hello, >> >> On Tue, Feb 05, 2019 at 01:40:27PM +0100, Fabrice Gasnier wrote: >>> Add suspend/resume PM sleep ops. When going to low power, disable >>> active PWM channel. Active PWM channel is resumed, by calling >>> pwm_apply_state(). This is inspired by Thierry's comment in [1]. >>> Don't touch inactive channels, as it may be used by other LPTimer MFD >>> child driver. >>> [1]https://lkml.org/lkml/2017/12/5/175 >>> >>> Signed-off-by: Fabrice Gasnier >>> --- >>> drivers/pwm/pwm-stm32-lp.c | 38 ++ >>> 1 file changed, 38 insertions(+) >>> >>> diff --git a/drivers/pwm/pwm-stm32-lp.c b/drivers/pwm/pwm-stm32-lp.c >>> index 0059b24c..0c40d48 100644 >>> --- a/drivers/pwm/pwm-stm32-lp.c >>> +++ b/drivers/pwm/pwm-stm32-lp.c >>> @@ -13,6 +13,7 @@ >>> #include >>> #include >>> #include >>> +#include >>> #include >>> #include >>> >>> @@ -20,6 +21,8 @@ struct stm32_pwm_lp { >>> struct pwm_chip chip; >>> struct clk *clk; >>> struct regmap *regmap; >>> + struct pwm_state suspend; >>> + bool suspended; >>> }; >>> >>> static inline struct stm32_pwm_lp *to_stm32_pwm_lp(struct pwm_chip *chip) >>> @@ -223,6 +226,40 @@ static int stm32_pwm_lp_remove(struct platform_device >>> *pdev) >>> return pwmchip_remove(&priv->chip); >>> } >>> >>> +#ifdef CONFIG_PM_SLEEP >>> +static int stm32_pwm_lp_suspend(struct device *dev) >>> +{ >>> + struct stm32_pwm_lp *priv = dev_get_drvdata(dev); >>> + >>> + pwm_get_state(&priv->chip.pwms[0], &priv->suspend); >>> + priv->suspended = priv->suspend.enabled; >>> + >>> + /* safe to call pwm_disable() for already disabled pwm */ >>> + pwm_disable(&priv->chip.pwms[0]); >>> + >>> + return pinctrl_pm_select_sleep_state(dev); >> >> IMHO a PWM should not stop if the PWM user didn't call pwm_disable() (or >> pwm_apply_state() with .enabled = false). >> >> I don't understand all the PM details, but I think there is no defined >> order in which devices are signalled to suspend. If so the PWM might be >> stopped before its consumer. Then the PWM changes state without the >> consumer being aware of that. >> >> I understand Thierry's position in the link you provided in the commit >> log consistant with my position here. > > Agreed, we should let users of the PWM take care of resuming the PWM in > the state and at the point in time that they require so. PWM users will > also likely do a pwm_disable() during their suspend implementation, so > doing this again in a PWM ->suspend() is not necessary, even if perhaps > harmless. > > So this leaves only the pinctrl_pm_select_sleep_state() call. That seems > fine, but I'm not sure that that's currently guaranteed to work. I think > we may need to implement device link support in the PWM framework to > guarantee the proper suspend/resume sequencing. Otherwise you may end up > in a situation where the PWM is actually suspended before the user and > glitch the pins before the user has a chance to disable the PWM. Hi Uwe, Thierry, I agree with both of you on the analysis. > > I think it'd be fine to merge the driver change here first before we add > device link support if this works for you. Just mentioning the issue > here in case you ever run into it. If you agree with the current approach, I can send a V2 with Tomasz's suggestion to remove the ifdefs and use __maybe_unused instead. Thanks for reviewing, Best Regards, Fabrice > > Thierry >
Re: [PATCH] mlx4_ib: Increase the timeout for CM cache
> On 5 Feb 2019, at 23:36, Jason Gunthorpe wrote: > > On Thu, Jan 31, 2019 at 06:09:51PM +0100, Håkon Bugge wrote: >> Using CX-3 virtual functions, either from a bare-metal machine or >> pass-through from a VM, MAD packets are proxied through the PF driver. >> >> Since the VMs have separate name spaces for MAD Transaction Ids >> (TIDs), the PF driver has to re-map the TIDs and keep the book keeping >> in a cache. >> >> Following the RDMA CM protocol, it is clear when an entry has to >> evicted form the cache. But life is not perfect, remote peers may die >> or be rebooted. Hence, it's a timeout to wipe out a cache entry, when >> the PF driver assumes the remote peer has gone. >> >> We have experienced excessive amount of DREQ retries during fail-over >> testing, when running with eight VMs per database server. >> >> The problem has been reproduced in a bare-metal system using one VM >> per physical node. In this environment, running 256 processes in each >> VM, each process uses RDMA CM to create an RC QP between himself and >> all (256) remote processes. All in all 16K QPs. >> >> When tearing down these 16K QPs, excessive DREQ retries (and >> duplicates) are observed. With some cat/paste/awk wizardry on the >> infiniband_cm sysfs, we observe: >> >> dreq: 5007 >> cm_rx_msgs: >> drep: 3838 >> dreq: 13018 >> rep: 8128 >> req: 8256 >> rtu: 8256 >> cm_tx_msgs: >> drep: 8011 >> dreq: 68856 >> rep: 8256 >> req: 8128 >> rtu: 8128 >> cm_tx_retries: >> dreq: 60483 >> >> Note that the active/passive side is distributed. >> >> Enabling pr_debug in cm.c gives tons of: >> >> [171778.814239] mlx4_ib_multiplex_cm_handler: id{slave: >> 1,sl_cm_id: 0xd393089f} is NULL! >> >> By increasing the CM_CLEANUP_CACHE_TIMEOUT from 5 to 30 seconds, the >> tear-down phase of the application is reduced from 113 to 67 >> seconds. Retries/duplicates are also significantly reduced: >> >> cm_rx_duplicates: >> dreq: 7726 >> [] >> cm_tx_retries: >> drep: 1 >> dreq: 7779 >> >> Increasing the timeout further didn't help, as these duplicates and >> retries stem from a too short CMA timeout, which was 20 (~4 seconds) >> on the systems. By increasing the CMA timeout to 22 (~17 seconds), the >> numbers fell down to about one hundred for both of them. >> >> Adjustment of the CMA timeout is _not_ part of this commit. >> >> Signed-off-by: Håkon Bugge >> --- >> drivers/infiniband/hw/mlx4/cm.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) > > Jack? What do you think? I am tempted to send a v2 making this a sysctl tuneable. This because, full-rack testing using 8 servers, each with 8 VMs, only showed 33% reduction in the occurrences of "mlx4_ib_multiplex_cm_handler: id{slave:1,sl_cm_id: 0xd393089f} is NULL" with this commit. But sure, Jack's opinion matters. Thxs, Håkon > >> diff --git a/drivers/infiniband/hw/mlx4/cm.c >> b/drivers/infiniband/hw/mlx4/cm.c >> index fedaf8260105..8c79a480f2b7 100644 >> --- a/drivers/infiniband/hw/mlx4/cm.c >> +++ b/drivers/infiniband/hw/mlx4/cm.c >> @@ -39,7 +39,7 @@ >> >> #include "mlx4_ib.h" >> >> -#define CM_CLEANUP_CACHE_TIMEOUT (5 * HZ) >> +#define CM_CLEANUP_CACHE_TIMEOUT (30 * HZ) >> >> struct id_map_entry { >> struct rb_node node; >> -- >> 2.20.1
Re: [PATCH -next] mm/compaction: no stuck in __reset_isolation_pfn()
On Tue, Feb 05, 2019 at 10:47:32PM -0500, Qian Cai wrote: > The commit c68d77911c23 ("mm, compaction: be selective about what > pageblocks to clear skip hints") introduced an infinite loop if a pfn is > invalid, it will loop again without increasing page counters. It can be > reproduced by running LTP tests on an arm64 server. > That was careless of me but the patch looks correct. Andrew, this is a fix to the mmotm patch mm-compaction-be-selective-about-what-pageblocks-to-clear-skip-hints.patch Acked-by: Mel Gorman Thanks Qian! -- Mel Gorman SUSE Labs
Re: [PATCH 05/32] timerfd/timens: Take into account ns clock offsets
On Wed, Feb 06, 2019 at 12:10:39AM +, Dmitry Safonov wrote: > From: Andrei Vagin > > Make timerfd respect timens offsets. > Provide two helpers timens_clock_to_host() timens_clock_from_host() that > are useful to wire up timens to different kernel subsystems. > Following patches will use timens_clock_from_host(), added here for > completeness. > > Signed-off-by: Andrei Vagin > Co-developed-by: Dmitry Safonov > Signed-off-by: Dmitry Safonov > --- > fs/timerfd.c | 16 +++- > 1 file changed, 11 insertions(+), 5 deletions(-) > > diff --git a/fs/timerfd.c b/fs/timerfd.c > index 803ca070d42e..c7ae1e371912 100644 > --- a/fs/timerfd.c > +++ b/fs/timerfd.c > @@ -26,6 +26,7 @@ > #include > #include > #include > +#include > > struct timerfd_ctx { > union { > @@ -433,22 +434,27 @@ SYSCALL_DEFINE2(timerfd_create, int, clockid, int, > flags) > } > > static int do_timerfd_settime(int ufd, int flags, > - const struct itimerspec64 *new, > + struct itimerspec64 *new, > struct itimerspec64 *old) > { > struct fd f; > struct timerfd_ctx *ctx; > int ret; > > - if ((flags & ~TFD_SETTIME_FLAGS) || > - !itimerspec64_valid(new)) > - return -EINVAL; Please don't defer this early test of a @flags value. Otherwise if @flags is invalid you continue fget/put/clock-to-host even if result will be dropped out then.
Re: [PATCH 2/4] pwm: stm32-lp: Add power management support
On Wed, Feb 06, 2019 at 09:42:48AM +0100, Fabrice Gasnier wrote: > If you agree with the current approach, I can send a V2 with Tomasz's > suggestion to remove the ifdefs and use __maybe_unused instead. I think the suspend callback should have something like: if (is_still_enabled) { /* * The consumer didn't stop us, so refuse to suspend. */ dev_err(dev, "The consumer didn't stop us, so refuse to suspend.\n"); return -EBUSY; } This way there are no bad surprises if the pwm is suspended before its consumer and it's obvious what is missing. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | http://www.pengutronix.de/ |
Re: [PATCH 05/32] timerfd/timens: Take into account ns clock offsets
On Wed, Feb 06, 2019 at 11:52:03AM +0300, Cyrill Gorcunov wrote: ... > > > > - if ((flags & ~TFD_SETTIME_FLAGS) || > > -!itimerspec64_valid(new)) > > - return -EINVAL; > > Please don't defer this early test of a @flags value. Otherwise > if @flags is invalid you continue fget/put/clock-to-host even > if result will be dropped out then. Just to clarify -- this could be done on top of the series to not resend the whole bunch.
Re: [PATCH v2 0/7] add support for SAM9X60 pin controller
On Thu, Jan 31, 2019 at 05:18:00PM +0100, Claudiu Beznea - M18063 wrote: > From: Claudiu Beznea Minor comment for patch 1. Acked-by: Ludovic Desroches for the whole series. > > This series adds drive strenght and slew rate support for SAMX60's pin > controller. For drive strenght we could have 2 values: low, high. > For slew rate we could have 2 values: enable, disabled. > > Besides this I took the chance and adapt the documentation for at91 pinctrl > driver. > > Changes in v2: > - s/microchip,sam9x60-pictrl/microchip,sam9x60-pinctrl in patches 3/7, 5/7 > > Claudiu Beznea (7): > pinctrl: at91: add option to use drive strength bits > pinctrl: at91: add drive strength support for SAM9X60 > pinctrl: at91: add compatibles for SAM9X60 pin controller > dt-bindings: add documentation for banks > dt-bindings: add bindings for SAM9X60 > pinctrl: at91: add slewrate support for SAM9X60 > dt-bindings: add documentation for slew rate > > .../bindings/pinctrl/atmel,at91-pinctrl.txt| 27 - > drivers/pinctrl/pinctrl-at91.c | 134 > +++-- > drivers/pinctrl/pinctrl-at91.h | 3 + > include/dt-bindings/pinctrl/at91.h | 4 + > 4 files changed, 155 insertions(+), 13 deletions(-) > > -- > 2.7.4 >
Re: [PATCH v5 0/7] Add Armada 3700 COMPHY support
Hi Kishon, Miquel Raynal wrote on Tue, 8 Jan 2019 17:31:17 +0100: > Hello, > > This series adds a new driver to support Armada 3700 COMPHY IP. > The series has been tested on an ESPRESSObin with SATA, PCIe > and USB3 host. For this purpose, patch 1 enumerates the SATA PHY > mode. The SGMII PHY mode that is supported by the IP has been written > (uses SMC calls anyway) but could not be tested on this platform. > > Three series will follow to add PHY support (or at least a PHY nodes > in the A3700/ESPRESSObin device trees) to the MVEBU AHCI driver, the > Aardvark PCI controller driver and the MVEBU xHCI driver. All this is > needed in order to achieve suspend to RAM on this platform. > > Thanks, > Miquèl > Any inputs on this series? It is important for me because it impacts SATA, PCIe and USB changes I plan to get merge. Thanks, Miquèl > > Changes since v4: > = > * Fixed error in the driver probe where PHY_INTERFACE_MODE_NA was > assigned to lane->mode instead of lane->submode. > * Compile-in the driver by default (depends on ARCH_MVEBU). > * Add dependency on HAVE_ARM_SMCC in Kconfig. > > Changes since v3: > = > * Added Rob's Reviewed-by tag on bindings. > * Rebased on top of phy -next as of the 12th of December, 2018. > * Adapted the driver to support SGMII/1000BASEX and > HS_SGMII/2500BASEX. > > Changes since v2: > = > * Remove redundant check on lane->port/args->arg[0] at the end of the > ->xlate() callback. Do it on both armada-cp110 and armada-a3700 > COMPHY drivers. > * Put my SoB as author of the patch first when a patch is co-developed. > > Changes since v1: > = > * Fix wrong check in ->xlate(). > * Apply the same fix to the cp110 comphy driver from which the a3700 > driver is based. > * Added credit to Gregorz Jaszczyk for his work. > * Added Suggested-by tag to the patch adding the COMPHY DT node. > > > Grzegorz Jaszczyk (1): > phy: enumerate SATA PHY mode > > Miquel Raynal (6): > phy: mvebu-cp110-comphy: fix port check in ->xlate() > phy: add A3700 COMPHY support > dt-bindings: phy: mvebu-comphy: extend the file to describe a3700 > bindings > MAINTAINERS: phy: add entry for Armada 3700 COMPHY driver > ARM64: dts: marvell: armada-37xx: fix SATA node scope > ARM64: dts: marvell: armada-37xx: declare the COMPHY node > > .../bindings/phy/phy-mvebu-comphy.txt | 65 +++- > MAINTAINERS | 6 + > arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 31 +- > drivers/phy/marvell/Kconfig | 12 + > drivers/phy/marvell/Makefile | 1 + > drivers/phy/marvell/phy-mvebu-a3700-comphy.c | 318 ++ > drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 2 - > include/linux/phy/phy.h | 1 + > 8 files changed, 421 insertions(+), 15 deletions(-) > create mode 100644 drivers/phy/marvell/phy-mvebu-a3700-comphy.c >
Re: [PATCH v7 2/3] arm64: implement ftrace with regs
Hi Torsten, On 18/01/2019 16:39, Torsten Duwe wrote: > Once gcc8 adds 2 NOPs at the beginning of each function, replace the > first NOP thus generated with a quick LR saver (move it to scratch reg > x9), so the 2nd replacement insn, the call to ftrace, does not clobber > the value. Ftrace will then generate the standard stack frames. > > Note that patchable-function-entry in GCC disables IPA-RA, which means > ABI register calling conventions are obeyed *and* scratch registers > such as x9 are available. > > Introduce and handle an ftrace_regs_trampoline for module PLTs, right > after ftrace_trampoline, and double the size of this special section. > > Signed-off-by: Torsten Duwe > > --- > > Mark, if you see your ftrace entry macro code being represented correctly > here, please add your sign-off, As I've initially copied it from your mail. > > --- > arch/arm64/include/asm/ftrace.h | 17 - > arch/arm64/include/asm/module.h |3 > arch/arm64/kernel/entry-ftrace.S | 125 > +-- > arch/arm64/kernel/ftrace.c | 114 ++- > arch/arm64/kernel/module-plts.c |3 > arch/arm64/kernel/module.c |2 > 6 files changed, 227 insertions(+), 37 deletions(-) [...] > --- a/arch/arm64/kernel/ftrace.c > +++ b/arch/arm64/kernel/ftrace.c > @@ -133,17 +163,45 @@ int ftrace_make_call(struct dyn_ftrace * > return ftrace_modify_code(pc, old, new, true); > } > > +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS > +int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr, > + unsigned long addr) > +{ > + unsigned long pc = rec->ip + REC_IP_BRANCH_OFFSET; > + u32 old, new; > + > + old = aarch64_insn_gen_branch_imm(pc, old_addr, true); > + new = aarch64_insn_gen_branch_imm(pc, addr, true); > + > + return ftrace_modify_code(pc, old, new, true); > +} > +#endif > + > /* > * Turn off the call to ftrace_caller() in instrumented function > */ > int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec, > unsigned long addr) > { > - unsigned long pc = rec->ip; > + unsigned long pc = rec->ip + REC_IP_BRANCH_OFFSET; Sorry to come back on this patch again, but I was looking at the ftrace code a bit, and I see that when processing the ftrace call locations, ftrace calls ftrace_call_adjust() on every ip registered as mcount caller (or in our case patchable entries). This ftrace_call_adjust() is arch specific, so I was thinking we could place the offset in here once and for all so we don't have to worry about it in the future. Also, I'm unsure whether it would be safe, but we could patch the "mov x9, lr" there as well. In theory, this would be called at init time (before secondary CPUs are brought up) and when loading a module (so I'd expect no-one is executing that code *yet*. If this is possible, I think it would make things a bit cleaner. Let me know if that would work. Thanks, -- Julien Thierry
Re: [PATCH 0/6] perf thread-stack: x86 retpolines
On 10/01/19 11:55 AM, Jiri Olsa wrote: > On Wed, Jan 09, 2019 at 11:18:29AM +0200, Adrian Hunter wrote: >> Hi >> >> Here are some patches to improve the exported call graph, primarily to deal >> with x86 retpolines. >> >> >> Adrian Hunter (6): >> perf tools: Fix split_kallsyms_for_kcore for trampoline symbols >> perf thread-stack: Tidy thread_stack__push_cp() usage >> perf thread-stack: Tidy thread_stack__no_call_return() by adding more >> local variables >> perf thread-stack: Represent jmps to the start of a different symbol >> perf thread-stack: Improve thread_stack__no_call_return() >> perf thread-stack: Hide x86 retpolines > > Acked-by: Jiri Olsa Can these be applied? > > thanks, > jirka > >> >> tools/perf/scripts/python/export-to-postgresql.py | 2 +- >> tools/perf/scripts/python/export-to-sqlite.py | 2 +- >> tools/perf/util/symbol.c | 2 + >> tools/perf/util/thread-stack.c| 235 >> ++ >> tools/perf/util/thread-stack.h| 3 + >> 5 files changed, 208 insertions(+), 36 deletions(-) >> >> >> Regards >> Adrian >
Re: [PATCH] net: stmmac: Variable "val" in function sun8i_dwmac_set_syscon() could be uninitialized
Hi, On Tue, Feb 05, 2019 at 02:15:59PM -0800, Yizhuo wrote: > In function sun8i_dwmac_set_syscon(), local variable "val" could > be uninitialized if function regmap_read() returns -EINVAL. > However, it will be used directly in the if statement, which > is potentially unsafe. > > Signed-off-by: Yizhuo > --- > drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 9 +++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c > b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c > index 39c2122a4f26..11d481c9e7ab 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c > @@ -639,9 +639,14 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv > *priv) > struct sunxi_priv_data *gmac = priv->plat->bsp_priv; > struct device_node *node = priv->device->of_node; > int ret; > - u32 reg, val; > + u32 reg, val = 0; I guess we don't need to initialize it anymore with the check you add? Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [RFC PATCH] clk: sunxi-ng: sun4i: Use CLK_SET_RATE_PARENT for mmc2 clock
On Tue, Feb 05, 2019 at 09:44:02PM +0800, Chen-Yu Tsai wrote: > On Tue, Feb 5, 2019 at 5:45 PM Maxime Ripard > wrote: > > > > On Sat, Feb 02, 2019 at 05:52:09PM +0200, Priit Laes wrote: > > > Recent patch of improving MP clock rate calculations by taking > > > into account whether adjusting parent rate is allowed, have > > > unfortunately broken eMMC support on A20 Olinuxino-Lime2-eMMC > > > boards which fail with following error: > > > > > > [snip] > > > EXT4-fs (mmcblk1p4): INFO: recovery required on readonly filesystem > > > EXT4-fs (mmcblk1p4): write access will be enabled during recovery > > > sunxi-mmc 1c11000.mmc: data error, sending stop command > > > sunxi-mmc 1c11000.mmc: send stop command failed > > > [/snip] > > > > > > Previously, mmc2 clock was requesting 520MHz and settling at 512MHz > > > clock rate with following parents: > > You mean 52 and 51.2 MHz. > > > > [snip] > > > pll-ddr-base2 20 76800 0 0 5 > > >pll-ddr-other1 10 76800 0 0 5 > > > mmc2 0 005120 0 0 5 > > > [/snip] > > > > > > Now, after the improvements, requested and settled rate are both > > > 520MHz, but as mmc2 clock cannot adjust parent rate, the situation > > > ends up like this: > > > [snip] > > > pll-periph-base 3 30 12 0 0 5 > > >pll-periph 6 60 6 0 0 5 > > > mmc2 3 305000 0 0 5 > > > [/snip] > > > > > > With this patch (allowing mmc2 to set parent rate), we end up with > > > working tree with both mmc0 (sd-card) and mmc2 (eMMC) working: > > > [snip] > > > pll-periph-base 3 30 31200 0 0 5 > > >mbus 1 107800 0 0 5 > > >pll-periph-sata 1 102600 0 0 5 > > > sata 1 102600 0 0 5 > > >pll-periph 5 50 15600 0 0 5 > > > mmc2 0 005200 0 0 5 > > > mmc0 0 003900 0 0 5 > > > [/snip] > > > > > > Fixes: 3f790433c3cb ("clk: sunxi-ng: Adjust MP clock parent rate when > > > allowed") > > > Signed-off-by: Priit Laes > > > > Applied, thanks! > > Maxime > > I'm concerned for other users of the PLL-PERIPH clock. AFAIK > all of them, except the HRTIMER, expect the clock rate to stay > the same and not change underneath them. And SATA expects it to > be at 600 MHz, as the datasheet says. And while it may not directly > apply to the LIME2, eMMC on newer SoCs / boards run at the slightly > reduced rate of 50 MHz just fine. > > In the commit in question, clocks without CLK_SET_RATE_PARENT > should be using the old code (now in the if conditional block), > i.e. the behavior should not have changed. > > I don't think this actually "fixes" whatever bug was introduced, > but only papers over the issue, and possible introduces further > issues for other users. You're right, I've overlooked that it was pll-periph being affected. I've dropped it for now. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: Question on handling managed IRQs when hotplugging CPUs
On 05/02/2019 18:23, Christoph Hellwig wrote: On Tue, Feb 05, 2019 at 03:09:28PM +, John Garry wrote: For SCSI devices, unfortunately not all IO sent to the HW originates from blk-mq or any other single entity. Where else would SCSI I/O originate from? Please note that I was referring to other management IO, like SAS SMP, TMFs, and other proprietary commands which the driver may generate for the HBA - https://marc.info/?l=linux-scsi&m=154831889001973&w=2 discusses some of them also. Thanks, John .
Re: [PATCH RESEND] PCI: Check for USB xHCI class for HAPS platform
Hi Bjorn, Am Dienstag, den 05.02.2019, 17:31 -0600 schrieb Bjorn Helgaas: > [+cc Richard, Lucas] > > On Tue, Feb 05, 2019 at 01:04:28PM -0800, Thinh Nguyen wrote: > > The Synopsys HAPS USB controller has a VID PID (16c3,abcd) that > > matches > > to an existing PCIe controller. This quirk is intended for USB HAPS > > devices only. To fix this, check for the PCI class USB xHCI to > > prevent > > matching the PCIe controllers. > > So there are at least three different parts with the same Vendor & > Device ID ([16c3:abcd]): > > 1) Synopsys HAPS USB3 controller > 2) Synopsys PCIe IP in the NXP i.MX6QP (reported by Lukas) The first incorporation of this IP in a Freescale SoC (i.MX6Q) also uses the device ID 0xabcd. So this seems to be a major miscommunication between NXP/Freescale and Synopsys. Regards, Lucas > 3) Synopsys PCIe IP in the NXP i.MX7D (reported by Trent) > > I don't know if Synopsys is to blame for 2 & 3, or if NXP was > expected > to change the Vendor ID when incorporating the Synopsys IP into the > i.MX designs. But even leaving the default Device ID of the PCIe IP > the same as another Synopsys device was probably a bad idea. > > dwc3-haps claims by Vendor & Device ID; it doesn't look at the Class > Code. What happens when it tries to claim the PCIe IP in i.MX? > > lspci also doesn't look at the Class Code when it looks up the text > name of devices, so it will print the same name for both. Of course, > the Linux PCI ID database at https://pci-ids.ucw.cz/read/PC/16c3 > shows > basically no information for the 0x16c3 Vendor ID, so it won't print > anything useful no matter what. You could add some things there :) > > > Fixes: 03e6742584af ("PCI: Override Synopsys USB 3.x HAPS device > > class") > > Signed-off-by: Thinh Nguyen > > --- > > drivers/pci/quirks.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > > index b0a413f3f7ca..e2a879e93d86 100644 > > --- a/drivers/pci/quirks.c > > +++ b/drivers/pci/quirks.c > > @@ -639,8 +639,9 @@ static void quirk_synopsys_haps(struct pci_dev > > *pdev) > > break; > > } > > } > > -DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SYNOPSYS, PCI_ANY_ID, > > - quirk_synopsys_haps); > > +DECLARE_PCI_FIXUP_CLASS_HEADER(PCI_VENDOR_ID_SYNOPSYS, PCI_ANY_ID, > > + PCI_CLASS_SERIAL_USB_XHCI, 0, > > + quirk_synopsys_haps); > > > > /* > > * Let's make the southbridge information explicit instead of > > having to > > -- > > 2.11.0 > >
Re: [PATCH v7 2/3] arm64: implement ftrace with regs
On 06/02/2019 08:59, Julien Thierry wrote: > Hi Torsten, > > On 18/01/2019 16:39, Torsten Duwe wrote: >> Once gcc8 adds 2 NOPs at the beginning of each function, replace the >> first NOP thus generated with a quick LR saver (move it to scratch reg >> x9), so the 2nd replacement insn, the call to ftrace, does not clobber >> the value. Ftrace will then generate the standard stack frames. >> >> Note that patchable-function-entry in GCC disables IPA-RA, which means >> ABI register calling conventions are obeyed *and* scratch registers >> such as x9 are available. >> >> Introduce and handle an ftrace_regs_trampoline for module PLTs, right >> after ftrace_trampoline, and double the size of this special section. >> >> Signed-off-by: Torsten Duwe >> >> --- >> >> Mark, if you see your ftrace entry macro code being represented correctly >> here, please add your sign-off, As I've initially copied it from your mail. >> >> --- >> arch/arm64/include/asm/ftrace.h | 17 - >> arch/arm64/include/asm/module.h |3 >> arch/arm64/kernel/entry-ftrace.S | 125 >> +-- >> arch/arm64/kernel/ftrace.c | 114 ++- >> arch/arm64/kernel/module-plts.c |3 >> arch/arm64/kernel/module.c |2 >> 6 files changed, 227 insertions(+), 37 deletions(-) > > [...] > >> --- a/arch/arm64/kernel/ftrace.c >> +++ b/arch/arm64/kernel/ftrace.c >> @@ -133,17 +163,45 @@ int ftrace_make_call(struct dyn_ftrace * >> return ftrace_modify_code(pc, old, new, true); >> } >> >> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS >> +int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr, >> +unsigned long addr) >> +{ >> +unsigned long pc = rec->ip + REC_IP_BRANCH_OFFSET; >> +u32 old, new; >> + >> +old = aarch64_insn_gen_branch_imm(pc, old_addr, true); >> +new = aarch64_insn_gen_branch_imm(pc, addr, true); >> + >> +return ftrace_modify_code(pc, old, new, true); >> +} >> +#endif >> + >> /* >> * Turn off the call to ftrace_caller() in instrumented function >> */ >> int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec, >> unsigned long addr) >> { >> -unsigned long pc = rec->ip; >> +unsigned long pc = rec->ip + REC_IP_BRANCH_OFFSET; > > Sorry to come back on this patch again, but I was looking at the ftrace > code a bit, and I see that when processing the ftrace call locations, > ftrace calls ftrace_call_adjust() on every ip registered as mcount > caller (or in our case patchable entries). This ftrace_call_adjust() is > arch specific, so I was thinking we could place the offset in here once > and for all so we don't have to worry about it in the future. > > Also, I'm unsure whether it would be safe, but we could patch the "mov > x9, lr" there as well. In theory, this would be called at init time > (before secondary CPUs are brought up) and when loading a module (so I'd > expect no-one is executing that code *yet*. > And if the above works, we could also define CC_HAVE_NOP_MCOUNT (when we have the patchable entry) and then we just not have to worry about the initial ftrace_make_nop() with MCOUNT_ADDR. -- Julien Thierry
[PATCH v7 00/10] support ROHM BD70528 PMIC
Patch series introducing support for ROHM BD70528 PMIC Please note that patch 1 breaks compilation without patches 2 and 3 Knowing the bd718x7 driver is already in upstream, it might be good if this change went through single tree, right? ROHM BD70528 is a programmable Power Management IC for battery powered 'ultra low power' systems like the pre-announced NXP i.MX7 ULP. This patch series introduces support for the PMIC. Please note that this driver only supports HW setup where PMIC is connected to I2C on A7 core. The other scenario is to use M4 as a power manager and connect pmic to M4. On such setups the A7 can only access pmic via M4 core using RPMSG virtio. Such setup depends on RPMSG implementation on M4 core and is currently not supported by this patch series. RTC block of the bd70528 can support 'wake' irq which wakes PMIC from standby state. Wake irq's can be armed to wake up system up to 24 hours from arming. bd70528 can also generate alarm interrupts which can be armed to occur years after triggering. The RTC driver does always arm both the waker and alarm irqs and does not utilize longer period of alarm interrupts. All the RTC timers are limited to occur within the next 24 hours. Any suggestions on more elegant timer support are welcome =) GPIO portion of bd70528 driver adds I/O support for driving GPIO pins or reading the state. The interrupt functionality is provided by regmap-irq. Current GPIO driver is not aware of whether the pin(s) are used for I/O or interrupts and it is up-to driver user to ensure there is no misconfiguration or "double use". The power-supply patch included in series is only poorly tested as I lack of hardware with real battery connected. Reset and ADC are not supported by this series. Changelog v7: Only patch 2 changed. - Avoid out-of-array-bounds access at regulator probe if unsupported chip type is passed to bd718x7 regulator driver. Changelog v6: Only patch 10 changed. - styling fixes pointed by Gunter Roeck - dropped RFC tag Changelog v5 (RFC): Only patch 7 changed. - Explained why lock is not needed at GPIO value getting - removed ampersands from function pointer assignments. Changelog v4 (RFC): patches 1,2,3,4,5,10 are unchanged from v3 DT-binding fixes suggested by Rob Herring: - drop interrupt-parent - drop clock-frequency - change pmic node name to a generic one RTC: - enable RTC block's irqs before registering rtc GPIO fixes after initial testing: - fix getting GPIO value when direction is output POWER: - Add ASCII art intended to clarify the charger HW state machine Changelog v3 (RFC): patches 1,2,3,4,5,6,7,8 and 10 are unchanged from v2 RTC fixups suggested by Guenter Roeck: - create bd70528_set_time_locked function in order to simplify error handling and to make mutex lock/unlock path more obvious - don't ignore errors on bd70528_set_time_locked - simplify bd70528_read_alarm enabled condition setting - add __packed to structs where members are mapped to HW registers - remove unnecessary brackets from enable condition in set_wake RTC: fixups suggested by Alessandro Belloni - don't use deprecated devm_rtc_device_register - add alarm_irq_enable callback - add range_min and range_max WDT: - add regmap and mutex pointers to WDT data so that they can be accessed without dereferencing the parent data - remove parent data pointer from WDT data - embed struct watchdog_device into WDT data in order to avoid double allocation. GPIO: - remove unused header as pointed by Linus Walleij POWER: - do not copy the whole MFD data (especially the mutex to avoid all possibilities of accidentally using the copy of a mutex) Changelog v2 (RFC): Mainly feedback from Guenter Roeck: - patches 1, 2, 3, 4, 5, 9 are unchanged. - mfd: own mutex for each bd70528 instance - embed in struct bd70528 - watchdog: do not copy parent device data - watchdog: fix deadlock caused by double locked mutex - watchdog: set initial timeouts and WDT parent information - watchdog: remove unnecessary ping function from ops - watchdog: and the comment regarding it - watchdog: allocate watchdog struct in order to allow multiple WDG instances - rtc: bd70528 fix the order of mutex unlock and re-enabling RTC based timers - rtc: fix the irq mask register address - power: fix the irq mask register address - regulator/regmap-irq: Drop the patches 1, 8 and 9 from original series as those were already applied by Mark Patch 1: split the bd718x7.h to generic and chip specific portions. (breaks compilation without patch 2 and 3) Patch 2: adapt bd718x7.h changes to bd718x7 regulator driver Patch 3: adapt bd718x7.h changes to bd718x7 clk driver Patch 4: add MFD core support for bd70528 Patch 5: support bd70528 clk using bd718x7 clk driver Patch 6: document DT bindings for BD70528 Patch 7: support BD70528 GPIO block Patch 8: support BD70528 RTC Patch 9: support BD70528 battery charger Patch 10: support BD705
[PATCH v7 01/10] mfd: bd718x7.h split to ROHM common and bd718x7 specific parts
Split the bd718x7.h to ROHM common and bd718x7 specific parts so that we do not need to add same things in every new ROHM PMIC header. Please note that this change requires changes also in bd718x7 sub-device drivers for regulators and clk. Signed-off-by: Matti Vaittinen --- drivers/mfd/rohm-bd718x7.c | 23 --- include/linux/mfd/rohm-bd718x7.h | 22 -- include/linux/mfd/rohm-generic.h | 20 3 files changed, 40 insertions(+), 25 deletions(-) create mode 100644 include/linux/mfd/rohm-generic.h diff --git a/drivers/mfd/rohm-bd718x7.c b/drivers/mfd/rohm-bd718x7.c index a29d529a96f4..7beb444a57cb 100644 --- a/drivers/mfd/rohm-bd718x7.c +++ b/drivers/mfd/rohm-bd718x7.c @@ -98,18 +98,19 @@ static int bd718xx_i2c_probe(struct i2c_client *i2c, return -ENOMEM; bd718xx->chip_irq = i2c->irq; - bd718xx->chip_type = (unsigned int)(uintptr_t) + bd718xx->chip.chip_type = (unsigned int)(uintptr_t) of_device_get_match_data(&i2c->dev); - bd718xx->dev = &i2c->dev; + bd718xx->chip.dev = &i2c->dev; dev_set_drvdata(&i2c->dev, bd718xx); - bd718xx->regmap = devm_regmap_init_i2c(i2c, &bd718xx_regmap_config); - if (IS_ERR(bd718xx->regmap)) { + bd718xx->chip.regmap = devm_regmap_init_i2c(i2c, + &bd718xx_regmap_config); + if (IS_ERR(bd718xx->chip.regmap)) { dev_err(&i2c->dev, "regmap initialization failed\n"); - return PTR_ERR(bd718xx->regmap); + return PTR_ERR(bd718xx->chip.regmap); } - ret = devm_regmap_add_irq_chip(&i2c->dev, bd718xx->regmap, + ret = devm_regmap_add_irq_chip(&i2c->dev, bd718xx->chip.regmap, bd718xx->chip_irq, IRQF_ONESHOT, 0, &bd718xx_irq_chip, &bd718xx->irq_data); if (ret) { @@ -118,7 +119,7 @@ static int bd718xx_i2c_probe(struct i2c_client *i2c, } /* Configure short press to 10 milliseconds */ - ret = regmap_update_bits(bd718xx->regmap, + ret = regmap_update_bits(bd718xx->chip.regmap, BD718XX_REG_PWRONCONFIG0, BD718XX_PWRBTN_PRESS_DURATION_MASK, BD718XX_PWRBTN_SHORT_PRESS_10MS); @@ -129,7 +130,7 @@ static int bd718xx_i2c_probe(struct i2c_client *i2c, } /* Configure long press to 10 seconds */ - ret = regmap_update_bits(bd718xx->regmap, + ret = regmap_update_bits(bd718xx->chip.regmap, BD718XX_REG_PWRONCONFIG1, BD718XX_PWRBTN_PRESS_DURATION_MASK, BD718XX_PWRBTN_LONG_PRESS_10S); @@ -149,7 +150,7 @@ static int bd718xx_i2c_probe(struct i2c_client *i2c, button.irq = ret; - ret = devm_mfd_add_devices(bd718xx->dev, PLATFORM_DEVID_AUTO, + ret = devm_mfd_add_devices(bd718xx->chip.dev, PLATFORM_DEVID_AUTO, bd718xx_mfd_cells, ARRAY_SIZE(bd718xx_mfd_cells), NULL, 0, regmap_irq_get_domain(bd718xx->irq_data)); @@ -162,11 +163,11 @@ static int bd718xx_i2c_probe(struct i2c_client *i2c, static const struct of_device_id bd718xx_of_match[] = { { .compatible = "rohm,bd71837", - .data = (void *)BD718XX_TYPE_BD71837, + .data = (void *)ROHM_CHIP_TYPE_BD71837, }, { .compatible = "rohm,bd71847", - .data = (void *)BD718XX_TYPE_BD71847, + .data = (void *)ROHM_CHIP_TYPE_BD71847, }, { } }; diff --git a/include/linux/mfd/rohm-bd718x7.h b/include/linux/mfd/rohm-bd718x7.h index fd194bfc836f..7f2dbde402a1 100644 --- a/include/linux/mfd/rohm-bd718x7.h +++ b/include/linux/mfd/rohm-bd718x7.h @@ -4,14 +4,9 @@ #ifndef __LINUX_MFD_BD718XX_H__ #define __LINUX_MFD_BD718XX_H__ +#include #include -enum { - BD718XX_TYPE_BD71837 = 0, - BD718XX_TYPE_BD71847, - BD718XX_TYPE_AMOUNT -}; - enum { BD718XX_BUCK1 = 0, BD718XX_BUCK2, @@ -321,18 +316,17 @@ enum { BD718XX_PWRBTN_LONG_PRESS_15S }; -struct bd718xx_clk; - struct bd718xx { - unsigned int chip_type; - struct device *dev; - struct regmap *regmap; - unsigned long int id; + /* +* Please keep this as the first member here as some +* drivers (clk) supporting more than one chip may only know this +* generic struct 'struct rohm_regmap_dev' and assume it is +* the first chunk of parent device's private data. +*/ + struct rohm_regmap_dev chip; int chip_irq; struct regmap_irq_chip_data *irq_data; - - struct bd718xx_clk *clk; }; #endif /* __LINUX_MFD_BD718XX_H__ */ diff --
Re: [PATCH v5 6/7] ARM64: dts: marvell: armada-37xx: fix SATA node scope
Hi Miquel, On mar., janv. 08 2019, Miquel Raynal wrote: > Fix the SATA IP memory area which is only 0x178 bytes long (from > Marvell A3700 specification). Actually, starting from the offset > 0xe0178, there is an area dedicated to the COMPHY driver. > > Suggested-by: Grzegorz Jaszczyk > Signed-off-by: Miquel Raynal As this one don't depend of the other part of the series I applied thi patches to mvebu/dt64. Thanks, Gregory > --- > arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi > b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi > index 4472bcd8f9fb..e7405931f381 100644 > --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi > +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi > @@ -358,7 +358,7 @@ > > sata: sata@e { > compatible = "marvell,armada-3700-ahci"; > - reg = <0xe 0x2000>; > + reg = <0xe 0x178>; > interrupts = ; > status = "disabled"; > }; > -- > 2.19.1 > -- Gregory Clement, Bootlin Embedded Linux and Kernel engineering http://bootlin.com
Re: [PATCH 1/9] pinctrl: qcom: ssbi-gpio: hardcode IRQ counts
Hi Brian! On Fri, Jan 25, 2019 at 5:23 PM Brian Masney wrote: > The probing of this driver calls platform_irq_count, which will > setup all of the IRQs that are configured in device tree. In > preparation for converting this driver to be a hierarchical IRQ > chip, hardcode the IRQ count based on the hardware type so that all > the IRQs are not configured immediately and are configured on an > as-needed basis later in the boot process. This change will also > allow for the removal of the interrupts property later in this > patch series once the hierarchical IRQ chip support is in. > > This patch also removes the generic qcom,ssbi-gpio OF match since we > don't know the number of pins. All of the existing upstream bindings > already include the more-specific binding. > > This change was not tested on any actual hardware, however the same > change was made to spmi-gpio and tested on a LG Nexus 5 (hammerhead) > phone. > > Signed-off-by: Brian Masney I found a bug here: > - pctrl->dev = &pdev->dev; Do not delete this line. Subsequent code makes heavy use of pctrl->dev and crashes. After fixing this I get other crashes :D but those are from other patches so I try to locate those problems too. Yours, Linus Walleij
[PATCH v7 03/10] clk: bd718x7: use chip specific and generic data structs
Header rohm-bd718x7.h was split to generic and component specific parts. This changed the struct bd718x7. Adapt the clk driver to these changes. Signed-off-by: Matti Vaittinen --- drivers/clk/clk-bd718x7.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/clk/clk-bd718x7.c b/drivers/clk/clk-bd718x7.c index 60422c72d142..461228ebf703 100644 --- a/drivers/clk/clk-bd718x7.c +++ b/drivers/clk/clk-bd718x7.c @@ -17,7 +17,7 @@ struct bd718xx_clk { u8 reg; u8 mask; struct platform_device *pdev; - struct bd718xx *mfd; + struct rohm_regmap_dev *mfd; }; static int bd71837_clk_set(struct clk_hw *hw, int status) @@ -68,7 +68,7 @@ static int bd71837_clk_probe(struct platform_device *pdev) int rval = -ENOMEM; const char *parent_clk; struct device *parent = pdev->dev.parent; - struct bd718xx *mfd = dev_get_drvdata(parent); + struct rohm_regmap_dev *mfd = dev_get_drvdata(parent); struct clk_init_data init = { .name = "bd718xx-32k-out", .ops = &bd71837_clk_ops, @@ -119,5 +119,5 @@ static struct platform_driver bd71837_clk = { module_platform_driver(bd71837_clk); MODULE_AUTHOR("Matti Vaittinen "); -MODULE_DESCRIPTION("BD71837 chip clk driver"); +MODULE_DESCRIPTION("BD71837/BD71847 chip clk driver"); MODULE_LICENSE("GPL"); -- 2.14.3 -- Matti Vaittinen, Linux device drivers ROHM Semiconductors, Finland SWDC Kiviharjunlenkki 1E 90220 OULU FINLAND ~~~ "I don't think so," said Rene Descartes. Just then, he vanished ~~~
[PATCH v7 02/10] regulator: bd718x7 use chip specific and generic data structs
Header rohm-bd718x7.h was split to generic and component specific parts. This changed the struct bd718x7. Adapt the regulator driver to these changes. Signed-off-by: Matti Vaittinen Acked-by: Mark Brown --- Please note that I kept the Acked-by from Mark as the change from struct bd718xx_pmic_inits pmic_regulators[] to struct bd718xx_pmic_inits pmic_regulators[ROHM_CHIP_TYPE_AMOUNT] seemed like trivial fix to me. Please let me know if this was not Ok. drivers/regulator/bd718x7-regulator.c | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/regulator/bd718x7-regulator.c b/drivers/regulator/bd718x7-regulator.c index ccea133778c8..524a87b991f3 100644 --- a/drivers/regulator/bd718x7-regulator.c +++ b/drivers/regulator/bd718x7-regulator.c @@ -1017,12 +1017,12 @@ static int bd718xx_probe(struct platform_device *pdev) { struct bd718xx *mfd; struct regulator_config config = { 0 }; - struct bd718xx_pmic_inits pmic_regulators[] = { - [BD718XX_TYPE_BD71837] = { + struct bd718xx_pmic_inits pmic_regulators[ROHM_CHIP_TYPE_AMOUNT] = { + [ROHM_CHIP_TYPE_BD71837] = { .r_datas = bd71837_regulators, .r_amount = ARRAY_SIZE(bd71837_regulators), }, - [BD718XX_TYPE_BD71847] = { + [ROHM_CHIP_TYPE_BD71847] = { .r_datas = bd71847_regulators, .r_amount = ARRAY_SIZE(bd71847_regulators), }, @@ -1037,15 +1037,15 @@ static int bd718xx_probe(struct platform_device *pdev) goto err; } - if (mfd->chip_type >= BD718XX_TYPE_AMOUNT || - !pmic_regulators[mfd->chip_type].r_datas) { + if (mfd->chip.chip_type >= ROHM_CHIP_TYPE_AMOUNT || + !pmic_regulators[mfd->chip.chip_type].r_datas) { dev_err(&pdev->dev, "Unsupported chip type\n"); err = -EINVAL; goto err; } /* Register LOCK release */ - err = regmap_update_bits(mfd->regmap, BD718XX_REG_REGLOCK, + err = regmap_update_bits(mfd->chip.regmap, BD718XX_REG_REGLOCK, (REGLOCK_PWRSEQ | REGLOCK_VREG), 0); if (err) { dev_err(&pdev->dev, "Failed to unlock PMIC (%d)\n", err); @@ -1065,7 +1065,7 @@ static int bd718xx_probe(struct platform_device *pdev) * for all reset types because OTP loading at READY will clear SEL * bit allowing HW defaults for power rails to be used */ - err = regmap_update_bits(mfd->regmap, BD718XX_REG_TRANS_COND1, + err = regmap_update_bits(mfd->chip.regmap, BD718XX_REG_TRANS_COND1, BD718XX_ON_REQ_POWEROFF_MASK | BD718XX_SWRESET_POWEROFF_MASK | BD718XX_WDOG_POWEROFF_MASK | @@ -1078,17 +1078,17 @@ static int bd718xx_probe(struct platform_device *pdev) dev_dbg(&pdev->dev, "Changed all resets from SVNS to READY\n"); } - for (i = 0; i < pmic_regulators[mfd->chip_type].r_amount; i++) { + for (i = 0; i < pmic_regulators[mfd->chip.chip_type].r_amount; i++) { const struct regulator_desc *desc; struct regulator_dev *rdev; const struct bd718xx_regulator_data *r; - r = &pmic_regulators[mfd->chip_type].r_datas[i]; + r = &pmic_regulators[mfd->chip.chip_type].r_datas[i]; desc = &r->desc; config.dev = pdev->dev.parent; - config.regmap = mfd->regmap; + config.regmap = mfd->chip.regmap; rdev = devm_regulator_register(&pdev->dev, desc, &config); if (IS_ERR(rdev)) { @@ -1104,7 +1104,7 @@ static int bd718xx_probe(struct platform_device *pdev) * can now switch the control from PMIC state machine to the * register interface */ - err = regmap_update_bits(mfd->regmap, r->init.reg, + err = regmap_update_bits(mfd->chip.regmap, r->init.reg, r->init.mask, r->init.val); if (err) { dev_err(&pdev->dev, @@ -1113,7 +1113,7 @@ static int bd718xx_probe(struct platform_device *pdev) goto err; } for (j = 0; j < r->additional_init_amnt; j++) { - err = regmap_update_bits(mfd->regmap, + err = regmap_update_bits(mfd->chip.regmap, r->additional_inits[j].reg, r->additional_inits[j].mask, r->additional_inits[j].val); -- 2.14.3 -- Matti Vaittinen, Linux device drivers ROHM Semiconductors, Finland SWDC Kiviharjunlenkki 1E 90220
[PATCH v7 04/10] mfd: bd70528: Support ROHM bd70528 PMIC - core
ROHM BD70528MWV is an ultra-low quiescent current general purpose single-chip power management IC for battery-powered portable devices. Add MFD core which enables chip access for following subdevices: - regulators/LED drivers - battery-charger - gpios - 32.768kHz clk - RTC - watchdog Signed-off-by: Matti Vaittinen --- drivers/mfd/Kconfig | 17 ++ drivers/mfd/Makefile | 1 + drivers/mfd/rohm-bd70528.c | 410 +++ include/linux/mfd/rohm-bd70528.h | 392 + 4 files changed, 820 insertions(+) create mode 100644 drivers/mfd/rohm-bd70528.c create mode 100644 include/linux/mfd/rohm-bd70528.h diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index f461460a2aeb..f1a0574cebb1 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -1847,6 +1847,23 @@ config MFD_ROHM_BD718XX NXP i.MX8. It contains 8 BUCK outputs and 7 LDOs, voltage monitoring and emergency shut down as well as 32,768KHz clock output. +config MFD_ROHM_BD70528 + tristate "ROHM BD70528 Power Management IC" + depends on I2C=y + depends on OF + select REGMAP_I2C + select REGMAP_IRQ + select MFD_CORE + help + Select this option to get support for the ROHM BD70528 Power + Management IC. BD71837 is general purpose single-chip power + management IC for battery-powered portable devices. It contains + 3 ultra-low current consumption buck converters, 3 LDOs and 2 LED + Drivers. Also included are 4 GPIOs, a real-time clock (RTC), a 32kHz + crystal oscillator, high-accuracy VREF for use with an external ADC, + 10 bits SAR ADC for battery temperature monitor and 1S battery + charger. + config MFD_STM32_LPTIMER tristate "Support for STM32 Low-Power Timer" depends on (ARCH_STM32 && OF) || COMPILE_TEST diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index 12980a4ad460..fc9b1408e39b 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -241,4 +241,5 @@ obj-$(CONFIG_MFD_MXS_LRADC) += mxs-lradc.o obj-$(CONFIG_MFD_SC27XX_PMIC) += sprd-sc27xx-spi.o obj-$(CONFIG_RAVE_SP_CORE) += rave-sp.o obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o +obj-$(CONFIG_MFD_ROHM_BD70528) += rohm-bd70528.o diff --git a/drivers/mfd/rohm-bd70528.c b/drivers/mfd/rohm-bd70528.c new file mode 100644 index ..580164addeeb --- /dev/null +++ b/drivers/mfd/rohm-bd70528.c @@ -0,0 +1,410 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +// +// Copyright (C) 2018 ROHM Semiconductors +// +// ROHM BD70528 PMIC driver + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define BD70528_INT_RES(_reg, _name) \ + { \ + .start = (_reg),\ + .end = (_reg), \ + .name = (_name),\ + .flags = IORESOURCE_IRQ,\ + } + +static const struct resource rtc_irqs[] = { + BD70528_INT_RES(BD70528_INT_RTC_ALARM, "bd70528-rtc-alm"), + BD70528_INT_RES(BD70528_INT_ELPS_TIM, "bd70528-elapsed-timer"), +}; + +static const struct resource charger_irqs[] = { + BD70528_INT_RES(BD70528_INT_BAT_OV_RES, "bd70528-bat-ov-res"), + BD70528_INT_RES(BD70528_INT_BAT_OV_DET, "bd70528-bat-ov-det"), + BD70528_INT_RES(BD70528_INT_DBAT_DET, "bd70528-bat-dead"), + BD70528_INT_RES(BD70528_INT_BATTSD_COLD_RES, "bd70528-bat-warmed"), + BD70528_INT_RES(BD70528_INT_BATTSD_COLD_DET, "bd70528-bat-cold"), + BD70528_INT_RES(BD70528_INT_BATTSD_HOT_RES, "bd70528-bat-cooled"), + BD70528_INT_RES(BD70528_INT_BATTSD_HOT_DET, "bd70528-bat-hot"), + BD70528_INT_RES(BD70528_INT_CHG_TSD, "bd70528-chg-tshd"), + BD70528_INT_RES(BD70528_INT_BAT_RMV, "bd70528-bat-removed"), + BD70528_INT_RES(BD70528_INT_BAT_DET, "bd70528-bat-detected"), + BD70528_INT_RES(BD70528_INT_DCIN2_OV_RES, "bd70528-dcin2-ov-res"), + BD70528_INT_RES(BD70528_INT_DCIN2_OV_DET, "bd70528-dcin2-ov-det"), + BD70528_INT_RES(BD70528_INT_DCIN2_RMV, "bd70528-dcin2-removed"), + BD70528_INT_RES(BD70528_INT_DCIN2_DET, "bd70528-dcin2-detected"), + BD70528_INT_RES(BD70528_INT_DCIN1_RMV, "bd70528-dcin1-removed"), + BD70528_INT_RES(BD70528_INT_DCIN1_DET, "bd70528-dcin1-detected"), +}; + +static struct mfd_cell bd70528_mfd_cells[] = { + { .name = "bd70528-pmic", }, + { .name = "bd70528-gpio", }, + /* +* We use BD71837 driver to drive the clk block. Only differences to +* BD70528 clock gate are the register address and mask. +*/ + { .name = "bd718xx-clk", }, + { .name = "bd70528-wdt", }, + { + .name = "bd70528-power", + .resources = &charger_irqs[0], +
[PATCH v7 05/10] clk: bd718x7: Support ROHM BD70528 clk block
ROHM BD70528 is an ultra low power PMIC with similar 32K clk as bd718x7. Only difference (from clk perspective) is register address. Add support for controlling BD70528 clk using bd718x7 driver. Signed-off-by: Matti Vaittinen --- drivers/clk/Kconfig | 6 +++--- drivers/clk/clk-bd718x7.c | 21 + 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index e5b2fe80eab4..992cfb86f2ca 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -285,10 +285,10 @@ config COMMON_CLK_STM32H7 config COMMON_CLK_BD718XX tristate "Clock driver for ROHM BD718x7 PMIC" - depends on MFD_ROHM_BD718XX + depends on MFD_ROHM_BD718XX || MFD_ROHM_BD70528 help - This driver supports ROHM BD71837 and ROHM BD71847 - PMICs clock gates. + This driver supports ROHM BD71837, ROHM BD71847 and + ROHM BD70528 PMICs clock gates. source "drivers/clk/actions/Kconfig" source "drivers/clk/bcm/Kconfig" diff --git a/drivers/clk/clk-bd718x7.c b/drivers/clk/clk-bd718x7.c index 461228ebf703..41c1374f4217 100644 --- a/drivers/clk/clk-bd718x7.c +++ b/drivers/clk/clk-bd718x7.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -86,9 +87,21 @@ static int bd71837_clk_probe(struct platform_device *pdev) dev_err(&pdev->dev, "No parent clk found\n"); return -EINVAL; } - - c->reg = BD718XX_REG_OUT32K; - c->mask = BD718XX_OUT32K_EN; + switch (mfd->chip_type) { + case ROHM_CHIP_TYPE_BD71837: + case ROHM_CHIP_TYPE_BD71847: + + c->reg = BD718XX_REG_OUT32K; + c->mask = BD718XX_OUT32K_EN; + break; + case ROHM_CHIP_TYPE_BD70528: + c->reg = BD70528_REG_CLK_OUT; + c->mask = BD70528_CLK_OUT_EN_MASK; + break; + default: + dev_err(&pdev->dev, "Unknown clk chip\n"); + return -EINVAL; + } c->mfd = mfd; c->pdev = pdev; c->hw.init = &init; @@ -119,5 +132,5 @@ static struct platform_driver bd71837_clk = { module_platform_driver(bd71837_clk); MODULE_AUTHOR("Matti Vaittinen "); -MODULE_DESCRIPTION("BD71837/BD71847 chip clk driver"); +MODULE_DESCRIPTION("BD71837/BD71847/BD70528 chip clk driver"); MODULE_LICENSE("GPL"); -- 2.14.3 -- Matti Vaittinen, Linux device drivers ROHM Semiconductors, Finland SWDC Kiviharjunlenkki 1E 90220 OULU FINLAND ~~~ "I don't think so," said Rene Descartes. Just then, he vanished ~~~
[PATCH v7 06/10] devicetree: bindings: Document first ROHM BD70528 bindings
Document bindings for regulators (3 bucks, 3 LDOs and 2 LED drivers) and 4 GPIO pins which can be configured for I/O or as interrupt sources withe configurable trigger levels. Signed-off-by: Matti Vaittinen Reviewed-by: Rob Herring Acked-by: Linus Walleij --- .../devicetree/bindings/mfd/rohm,bd70528-pmic.txt | 102 + 1 file changed, 102 insertions(+) create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd70528-pmic.txt diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd70528-pmic.txt b/Documentation/devicetree/bindings/mfd/rohm,bd70528-pmic.txt new file mode 100644 index ..f80be7e40184 --- /dev/null +++ b/Documentation/devicetree/bindings/mfd/rohm,bd70528-pmic.txt @@ -0,0 +1,102 @@ +* ROHM BD70528 Power Management Integrated Circuit bindings + +BD70528MWV is an ultra-low Iq general purpose single-chip power management IC +for battery-powered portable devices. The IC integrates 3 ultra-low current +consumption buck converters, 3 LDOs and 2 LED Drivers. Also included are 4 +GPIOs, a real-time clock (RTC), a 32kHz clock gate, high-accuracy VREF +for use with an external ADC, flexible dual-input power path, 10 bits SAR ADC +for battery temperature monitor and 1S battery charger with scalable charge +currents. + +Required properties: + - compatible : Should be "rohm,bd70528" + - reg : I2C slave address. + - interrupts : The interrupt line the device is connected to. + - interrupt-controller: To indicate bd70528 acts as an interrupt controller. + - #interrupt-cells: Should be 2. usage is compliant to the 2 cells + variant of ../interrupt-controller/interrupts.txt + - gpio-controller : To indicate bd70528 acts as a gpio controller. + - #gpio-cells : Should be 2. The first cell is the pin number and + the second cell is used to specify flags. See + ../gpio/gpio.txt for more information. + - #clock-cells: Should be 0. + - regulators: : List of child nodes that specify the regulators. + Please see ../regulator/rohm,bd70528-regulator.txt + +Optional properties: + - clock-output-names : Should contain name for output clock. + +Example: +/* external oscillator */ +osc: oscillator { + compatible = "fixed-clock"; + #clock-cells = <1>; + clock-frequency = <32768>; + clock-output-names = "osc"; +}; + +pmic: pmic@4b { + compatible = "rohm,bd70528"; + reg = <0x4b>; + interrupt-parent = <&gpio1>; + interrupts = <29 GPIO_ACTIVE_LOW>; + clocks = <&osc 0>; + #clock-cells = <0>; + clock-output-names = "bd70528-32k-out"; + #gpio-cells = <2>; + gpio-controller; + interrupt-controller; + #interrupt-cells = <2>; + + regulators { + buck1: BUCK1 { + regulator-name = "buck1"; + regulator-min-microvolt = <120>; + regulator-max-microvolt = <340>; + regulator-boot-on; + regulator-ramp-delay = <125>; + }; + buck2: BUCK2 { + regulator-name = "buck2"; + regulator-min-microvolt = <120>; + regulator-max-microvolt = <330>; + regulator-boot-on; + regulator-ramp-delay = <125>; + }; + buck3: BUCK3 { + regulator-name = "buck3"; + regulator-min-microvolt = <80>; + regulator-max-microvolt = <180>; + regulator-boot-on; + regulator-ramp-delay = <250>; + }; + ldo1: LDO1 { + regulator-name = "ldo1"; + regulator-min-microvolt = <165>; + regulator-max-microvolt = <330>; + regulator-boot-on; + }; + ldo2: LDO2 { + regulator-name = "ldo2"; + regulator-min-microvolt = <165>; + regulator-max-microvolt = <330>; + regulator-boot-on; + }; + + ldo3: LDO3 { + regulator-name = "ldo3"; + regulator-min-microvolt = <165>; + regulator-max-microvolt = <330>; + }; + led_ldo1: LED_LDO1 { + regulator-name = "led_ldo1"; + regulator-min-microvolt = <20>; + regulator-max-microvolt = <30>; + }; + led_ldo2: LED_LDO2 { + regulator-name = "led_ldo2"; + regulator-min-microvolt = <20>; + regulator-max-
[PATCH v7 07/10] gpio: Initial support for ROHM bd70528 GPIO block
ROHM BD70528 PMIC has 4 GPIO pins. Allow them to be controlled by GPIO framework. IRQs are handled by regmap-irq and GPIO driver is not aware of the irq usage. Signed-off-by: Matti Vaittinen Reviewed-by: Linus Walleij --- drivers/gpio/Kconfig| 11 +++ drivers/gpio/Makefile | 1 + drivers/gpio/gpio-bd70528.c | 232 3 files changed, 244 insertions(+) create mode 100644 drivers/gpio/gpio-bd70528.c diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index b5a2845347ec..c82187648630 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -962,6 +962,17 @@ config GPIO_ARIZONA help Support for GPIOs on Wolfson Arizona class devices. +config GPIO_BD70528 + tristate "ROHM BD70528 GPIO support" + depends on MFD_ROHM_BD70528 + help + Support for GPIOs on ROHM BD70528 PMIC. There are four GPIOs + available on the ROHM PMIC in total. The GPIOs can also + generate interrupts. + + This driver can also be built as a module. If so, the module + will be called gpio-bd70528. + config GPIO_BD9571MWV tristate "ROHM BD9571 GPIO support" depends on MFD_BD9571MWV diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index 37628f8dbf70..6f12c83598fc 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -34,6 +34,7 @@ obj-$(CONFIG_GPIO_ATH79) += gpio-ath79.o obj-$(CONFIG_GPIO_ASPEED) += gpio-aspeed.o obj-$(CONFIG_GPIO_RASPBERRYPI_EXP) += gpio-raspberrypi-exp.o obj-$(CONFIG_GPIO_BCM_KONA)+= gpio-bcm-kona.o +obj-$(CONFIG_GPIO_BD70528) += gpio-bd70528.o obj-$(CONFIG_GPIO_BD9571MWV) += gpio-bd9571mwv.o obj-$(CONFIG_GPIO_BRCMSTB) += gpio-brcmstb.o obj-$(CONFIG_GPIO_BT8XX) += gpio-bt8xx.o diff --git a/drivers/gpio/gpio-bd70528.c b/drivers/gpio/gpio-bd70528.c new file mode 100644 index ..c00d5d67dd74 --- /dev/null +++ b/drivers/gpio/gpio-bd70528.c @@ -0,0 +1,232 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (C) 2018 ROHM Semiconductors +// gpio-bd70528.c ROHM BD70528MWV gpio driver + +#include +#include +#include +#include +#include +#include + +#define GPIO_IN_REG(offset) (BD70528_REG_GPIO1_IN + offset*2) +#define GPIO_OUT_REG(offset) (BD70528_REG_GPIO1_OUT + offset*2) + +struct bd70528_gpio { + struct rohm_regmap_dev chip; + struct gpio_chip gpio; +}; + +static int bd70528_set_debounce(struct bd70528_gpio *bdgpio, + unsigned int offset, unsigned int debounce) +{ + u8 val; + + switch (debounce) { + case 0: + val = BD70528_DEBOUNCE_DISABLE; + break; + case 1 ... 15: + val = BD70528_DEBOUNCE_15MS; + break; + case 16 ... 30: + val = BD70528_DEBOUNCE_30MS; + break; + case 31 ... 50: + val = BD70528_DEBOUNCE_50MS; + break; + default: + dev_err(bdgpio->chip.dev, + "Invalid debouce value %u\n", debounce); + return -EINVAL; + } + return regmap_update_bits(bdgpio->chip.regmap, GPIO_IN_REG(offset), +BD70528_DEBOUNCE_MASK, val); +} + +static int bd70528_get_direction(struct gpio_chip *chip, unsigned int offset) +{ + struct bd70528_gpio *bdgpio = gpiochip_get_data(chip); + int val, ret; + + /* Do we need to do something to IRQs here? */ + ret = regmap_read(bdgpio->chip.regmap, GPIO_OUT_REG(offset), &val); + if (ret) { + dev_err(bdgpio->chip.dev, "Could not read gpio direction\n"); + return ret; + } + + return !(val & BD70528_GPIO_OUT_EN_MASK); +} + +static int bd70528_gpio_set_config(struct gpio_chip *chip, unsigned int offset, + unsigned long config) +{ + struct bd70528_gpio *bdgpio = gpiochip_get_data(chip); + + switch (pinconf_to_config_param(config)) { + case PIN_CONFIG_DRIVE_OPEN_DRAIN: + return regmap_update_bits(bdgpio->chip.regmap, + GPIO_OUT_REG(offset), + BD70528_GPIO_DRIVE_MASK, + BD70528_GPIO_OPEN_DRAIN); + break; + case PIN_CONFIG_DRIVE_PUSH_PULL: + return regmap_update_bits(bdgpio->chip.regmap, + GPIO_OUT_REG(offset), + BD70528_GPIO_DRIVE_MASK, + BD70528_GPIO_PUSH_PULL); + break; + case PIN_CONFIG_INPUT_DEBOUNCE: + return bd70528_set_debounce(bdgpio, offset, + pinconf_to_config_argument(config)); + break; + default: + break; + } + return -ENOTSUPP; +} + +static int bd70528_di
[PATCH v7 08/10] rtc: bd70528: Initial support for ROHM bd70528 RTC
Support RTC block in ROHM bd70528 power management IC. Support getting and setting the time and date as well as arming an alarm which can also be used to wake the PMIC from standby state. HW supports wake interrupt only for the next 24 hours (sec, minute and hour information only) so we limit also the alarm interrupt to this 24 hours for the sake of consistency. Signed-off-by: Matti Vaittinen Acked-by: Alexandre Belloni --- drivers/rtc/Kconfig | 8 + drivers/rtc/Makefile | 1 + drivers/rtc/rtc-bd70528.c | 500 ++ 3 files changed, 509 insertions(+) create mode 100644 drivers/rtc/rtc-bd70528.c diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig index 225b0b8516f3..df6211cbd83f 100644 --- a/drivers/rtc/Kconfig +++ b/drivers/rtc/Kconfig @@ -487,6 +487,14 @@ config RTC_DRV_M41T80_WDT help If you say Y here you will get support for the watchdog timer in the ST M41T60 and M41T80 RTC chips series. +config RTC_DRV_BD70528 + tristate "ROHM BD70528 PMIC RTC" + help + If you say Y here you will get support for the RTC + on ROHM BD70528 Power Management IC. + + This driver can also be built as a module. If so, the module + will be called rtc-bd70528. config RTC_DRV_BQ32K tristate "TI BQ32000" diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile index df022d820bee..740b13840913 100644 --- a/drivers/rtc/Makefile +++ b/drivers/rtc/Makefile @@ -36,6 +36,7 @@ obj-$(CONFIG_RTC_DRV_ASM9260) += rtc-asm9260.o obj-$(CONFIG_RTC_DRV_AT91RM9200)+= rtc-at91rm9200.o obj-$(CONFIG_RTC_DRV_AT91SAM9) += rtc-at91sam9.o obj-$(CONFIG_RTC_DRV_AU1XXX) += rtc-au1xxx.o +obj-$(CONFIG_RTC_DRV_BD70528) += rtc-bd70528.o obj-$(CONFIG_RTC_DRV_BQ32K)+= rtc-bq32k.o obj-$(CONFIG_RTC_DRV_BQ4802) += rtc-bq4802.o obj-$(CONFIG_RTC_DRV_BRCMSTB) += rtc-brcmstb-waketimer.o diff --git a/drivers/rtc/rtc-bd70528.c b/drivers/rtc/rtc-bd70528.c new file mode 100644 index ..2917a2fe685e --- /dev/null +++ b/drivers/rtc/rtc-bd70528.c @@ -0,0 +1,500 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +// +// Copyright (C) 2018 ROHM Semiconductors +// +// RTC driver for ROHM BD70528 PMIC + +#include +#include +#include +#include +#include +#include +#include + +/* + * We read regs RTC_SEC => RTC_YEAR + * this struct is ordered according to chip registers. + * Keep it u8 only to avoid padding issues. + */ +struct bd70528_rtc_day { + u8 sec; + u8 min; + u8 hour; +} __packed; + +struct bd70528_rtc_data { + struct bd70528_rtc_day time; + u8 week; + u8 day; + u8 month; + u8 year; +} __packed; + +struct bd70528_rtc_wake { + struct bd70528_rtc_day time; + u8 ctrl; +} __packed; + +struct bd70528_rtc_alm { + struct bd70528_rtc_data data; + u8 alm_mask; + u8 alm_repeat; +} __packed; + +struct bd70528_rtc { + struct bd70528 *mfd; + struct device *dev; +}; + +static int bd70528_set_wake(struct bd70528 *bd70528, + int enable, int *old_state) +{ + int ret; + unsigned int ctrl_reg; + + ret = regmap_read(bd70528->chip.regmap, BD70528_REG_WAKE_EN, &ctrl_reg); + if (ret) + return ret; + + if (old_state) { + if (ctrl_reg & BD70528_MASK_WAKE_EN) + *old_state |= BD70528_WAKE_STATE_BIT; + else + *old_state &= ~BD70528_WAKE_STATE_BIT; + + if (!enable == !(*old_state & BD70528_WAKE_STATE_BIT)) + return 0; + } + + if (enable) + ctrl_reg |= BD70528_MASK_WAKE_EN; + else + ctrl_reg &= ~BD70528_MASK_WAKE_EN; + + return regmap_write(bd70528->chip.regmap, BD70528_REG_WAKE_EN, + ctrl_reg); +} + +static int bd70528_set_elapsed_tmr(struct bd70528 *bd70528, + int enable, int *old_state) +{ + int ret; + unsigned int ctrl_reg; + + /* +* TBD +* What is the purpose of elapsed timer ? +* Is the timeout registers counting down, or is the disable - re-enable +* going to restart the elapsed-time counting? If counting is restarted +* the timeout should be decreased by the amount of time that has +* elapsed since starting the timer. Maybe we should store the monotonic +* clock value when timer is started so that if RTC is set while timer +* is armed we could do the compensation. This is a hack if RTC/system +* clk are drifting. OTOH, RTC controlled via I2C is in any case +* inaccurate... +*/ + ret = regmap_read(bd70528->chip.regmap, BD70528_REG_ELAPSED_TIMER_EN, + &ctrl_reg); + if (ret) + return ret; + + if (old_state) { + if (ctrl_reg & BD70528_MASK_ELAPSED_TIMER_EN) +
[PATCH v7 09/10] power: supply: Initial support for ROHM BD70528 PMIC charger block
ROHM BD70528 PMIC includes battery charger block. Support charger staus queries and doing few basic settings like input current limit and charging current. Signed-off-by: Matti Vaittinen --- drivers/power/supply/Kconfig | 9 + drivers/power/supply/Makefile | 1 + drivers/power/supply/bd70528-charger.c | 738 + 3 files changed, 748 insertions(+) create mode 100644 drivers/power/supply/bd70528-charger.c diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig index e901b9879e7e..903c97a67bf0 100644 --- a/drivers/power/supply/Kconfig +++ b/drivers/power/supply/Kconfig @@ -660,4 +660,13 @@ config FUEL_GAUGE_SC27XX Say Y here to enable support for fuel gauge with SC27XX PMIC chips. +config CHARGER_BD70528 + tristate "ROHM bd70528 charger driver" + depends on MFD_ROHM_BD70528 + default n + help +Say Y here to enable support for getting battery status +information and altering charger configurations from charger +block of the ROHM BD70528 Power Management IC. + endif # POWER_SUPPLY diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile index b731c2a9b695..c60387b04bfa 100644 --- a/drivers/power/supply/Makefile +++ b/drivers/power/supply/Makefile @@ -87,3 +87,4 @@ obj-$(CONFIG_AXP288_CHARGER) += axp288_charger.o obj-$(CONFIG_CHARGER_CROS_USBPD) += cros_usbpd-charger.o obj-$(CONFIG_CHARGER_SC2731) += sc2731_charger.o obj-$(CONFIG_FUEL_GAUGE_SC27XX)+= sc27xx_fuel_gauge.o +obj-$(CONFIG_CHARGER_BD70528) += bd70528-charger.o diff --git a/drivers/power/supply/bd70528-charger.c b/drivers/power/supply/bd70528-charger.c new file mode 100644 index ..db2824a9d960 --- /dev/null +++ b/drivers/power/supply/bd70528-charger.c @@ -0,0 +1,738 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +// +// Copyright (C) 2018 ROHM Semiconductors +// +// power-supply driver for ROHM BD70528 PMIC + +/* + * BD70528 charger HW state machine. + * + * The thermal shutdown state is not drawn. From any other state but + * battery error and suspend it is possible to go to TSD/TMP states + * if temperature is out of bounds. + * + * CHG_RST = H + * or CHG_EN=L + * or (DCIN2_UVLO=L && DCIN1_UVLO=L) + * or (DCIN2_OVLO=H & DCIN1_UVKLO=L) + * + * +--+ +--+ + * | | | | + * | Any state +---> |Suspend | + * | | | | + * +--+ +--+---+ + * | + * CHG_EN = H && BAT_DET = H &&| + * No errors (temp, bat_ov, UVLO, | + * OVLO...)| + * | + * BAT_OV or +-v--+ + * (DBAT && TTRI)|| + * +-+ Trickle Charge | <---+ + * | || | + * | +---++ | + * | | | + * | | ^| + * |V_BAT > VTRI_TH | | VBAT < VTRI_TH - 50mV | + * | | || + * | v || + * | || + * | BAT_OV or +--++ | + * | (DBAT && TFST) | | | + * | ++ Fast Charge | | + * | || | | + * v v++--+ | + *| | + *++ ILIM_DET=L |^ ILIM_DET| + *|| & CV_DET=H || or CV_DET=L | + *| Battery Error | & VBAT > || or VBAT < VRECHG_TH | + *|| VRECHG_TH || or IBAT > IFST/x | + *++ & IBAT < || | + * IFST/x v| | + * ^ | | + * | +-+-+ | + * | | | | + * +---+ Top OFF | | + * BAT_OV = H or| | | + * (DBAT && TFST) +-+-+ | + * | | + * Stay top-off for 15s | | + * v | + * | + *
[PATCH v7 10/10] watchdog: bd70528: Initial support for ROHM BD70528 watchdog block
Initial support for watchdog block included in ROHM BD70528 power management IC. Configurations for low power states are still to be checked. Signed-off-by: Matti Vaittinen Acked-by: Guenter Roeck --- drivers/watchdog/Kconfig | 12 +++ drivers/watchdog/Makefile | 1 + drivers/watchdog/bd70528_wdt.c | 190 + 3 files changed, 203 insertions(+) create mode 100644 drivers/watchdog/bd70528_wdt.c diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index 57f017d74a97..f30e3a3e886e 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -90,6 +90,18 @@ config SOFT_WATCHDOG_PRETIMEOUT watchdog. Be aware that governors might affect the watchdog because it is purely software, e.g. the panic governor will stall it! +config BD70528_WATCHDOG + tristate "ROHM BD70528 PMIC Watchdog" + depends on MFD_ROHM_BD70528 + select WATCHDOG_CORE + help + Support for the watchdog in the ROHM BD70528 PMIC. Watchdog trigger + cause system reset. + + Say Y here to include support for the ROHM BD70528 watchdog. + Alternatively say M to compile the driver as a module, + which will be called bd70528_wdt. + config DA9052_WATCHDOG tristate "Dialog DA9052 Watchdog" depends on PMIC_DA9052 || COMPILE_TEST diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index a0917ef28e07..1ce87a3b1172 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -204,6 +204,7 @@ obj-$(CONFIG_WATCHDOG_SUN4V)+= sun4v_wdt.o obj-$(CONFIG_XEN_WDT) += xen_wdt.o # Architecture Independent +obj-$(CONFIG_BD70528_WATCHDOG) += bd70528_wdt.o obj-$(CONFIG_DA9052_WATCHDOG) += da9052_wdt.o obj-$(CONFIG_DA9055_WATCHDOG) += da9055_wdt.o obj-$(CONFIG_DA9062_WATCHDOG) += da9062_wdt.o diff --git a/drivers/watchdog/bd70528_wdt.c b/drivers/watchdog/bd70528_wdt.c new file mode 100644 index ..cab802ddde06 --- /dev/null +++ b/drivers/watchdog/bd70528_wdt.c @@ -0,0 +1,190 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (C) 2018 ROHM Semiconductors +// ROHM BD70528MWV watchdog driver + +#include +#include +#include +#include +#include +#include +#include +#include + +/* + * Max time we can set is 1 hour, 59 minutes and 59 seconds + * and Minimum time is 1 second + */ +#define WDT_MAX_MS ((2 * 60 * 60 - 1) * 1000) +#define WDT_MIN_MS 1000 +#define DEFAULT_TIMEOUT60 + +struct wdtbd70528 { + struct device *dev; + struct regmap *regmap; + struct mutex *rtc_lock; + struct watchdog_device wdt; +}; + +static int bd70528_wdt_set_locked(struct wdtbd70528 *w, int enable) +{ + struct bd70528 *bd70528; + + bd70528 = container_of(w->rtc_lock, struct bd70528, rtc_timer_lock); + return bd70528->wdt_set(bd70528, enable, NULL); +} + +static int bd70528_wdt_set(struct wdtbd70528 *w, int enable) +{ + int ret; + + mutex_lock(w->rtc_lock); + ret = bd70528_wdt_set_locked(w, enable); + mutex_unlock(w->rtc_lock); + + return ret; +} + +static int bd70528_wdt_start(struct watchdog_device *wdt) +{ + struct wdtbd70528 *w = watchdog_get_drvdata(wdt); + + dev_dbg(w->dev, "WDT ping...\n"); + return bd70528_wdt_set(w, 1); +} + +static int bd70528_wdt_stop(struct watchdog_device *wdt) +{ + struct wdtbd70528 *w = watchdog_get_drvdata(wdt); + + dev_dbg(w->dev, "WDT stopping...\n"); + return bd70528_wdt_set(w, 0); +} + +static int bd70528_wdt_set_timeout(struct watchdog_device *wdt, + unsigned int timeout) +{ + unsigned int hours; + unsigned int minutes; + unsigned int seconds; + int ret; + struct wdtbd70528 *w = watchdog_get_drvdata(wdt); + + seconds = timeout; + hours = timeout / (60 * 60); + /* Maximum timeout is 1h 59m 59s => hours is 1 or 0 */ + if (hours) + seconds -= (60 * 60); + minutes = seconds / 60; + seconds = seconds % 60; + + mutex_lock(w->rtc_lock); + + ret = bd70528_wdt_set_locked(w, 0); + if (ret) + goto out_unlock; + + ret = regmap_update_bits(w->regmap, BD70528_REG_WDT_HOUR, +BD70528_MASK_WDT_HOUR, hours); + if (ret) { + dev_err(w->dev, "Failed to set WDT hours\n"); + goto out_en_unlock; + } + ret = regmap_update_bits(w->regmap, BD70528_REG_WDT_MINUTE, +BD70528_MASK_WDT_MINUTE, bin2bcd(minutes)); + if (ret) { + dev_err(w->dev, "Failed to set WDT minutes\n"); + goto out_en_unlock; + } + ret = regmap_update_bits(w->regmap, BD70528_REG_WDT_SEC, +BD70528_MASK_WDT_SEC, bin2bcd(seconds)); + if (ret) + dev_err(w->dev, "Failed to set WDT seconds\n"); + else +
Re: [PATCH v2 1/7] pinctrl: at91: add option to use drive strength bits
On 06.02.2019 10:13, Ludovic Desroches wrote: > On Thu, Jan 31, 2019 at 05:18:04PM +0100, Claudiu Beznea - M18063 wrote: >> From: Claudiu Beznea >> >> SAM9X60 uses high and low drive strengths. To implement this, in >> at91_pinctrl_mux_ops::set_drivestrength and >> at91_pinctrl_mux_ops::get_drivestrength we need bit numbers of >> drive strengths (1 for low, 2 for high), thus change the code to >> allow the usage of drive strength bit numbers. >> >> Signed-off-by: Claudiu Beznea >> --- >> drivers/pinctrl/pinctrl-at91.c | 32 >> 1 file changed, 20 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c >> index 3d49bbbcdbc7..31f06dafca2e 100644 >> --- a/drivers/pinctrl/pinctrl-at91.c >> +++ b/drivers/pinctrl/pinctrl-at91.c >> @@ -72,10 +72,15 @@ static int gpio_banks; >> * DRIVE_STRENGTH_DEFAULT is just a placeholder to avoid changing the drive >> * strength when there is no dt config for it. >> */ >> -#define DRIVE_STRENGTH_DEFAULT (0 << DRIVE_STRENGTH_SHIFT) >> -#define DRIVE_STRENGTH_LOW (1 << DRIVE_STRENGTH_SHIFT) >> -#define DRIVE_STRENGTH_MED (2 << DRIVE_STRENGTH_SHIFT) >> -#define DRIVE_STRENGTH_HI (3 << DRIVE_STRENGTH_SHIFT) >> +enum drive_strength_bit { >> +DRIVE_STRENGTH_BIT_DEF, >> +DRIVE_STRENGTH_BIT_LOW, >> +DRIVE_STRENGTH_BIT_MED, >> +DRIVE_STRENGTH_BIT_HI, >> +}; >> + >> +#define DRIVE_STRENGTH_BIT_MSK(name)(DRIVE_STRENGTH_BIT_##name << \ >> + DRIVE_STRENGTH_SHIFT) >> >> /** >> * struct at91_pmx_func - describes AT91 pinmux functions >> @@ -551,7 +556,7 @@ static unsigned at91_mux_sama5d3_get_drivestrength(void >> __iomem *pio, >> /* SAMA5 strength is 1:1 with our defines, >> * except 0 is equivalent to low per datasheet */ >> if (!tmp) >> -tmp = DRIVE_STRENGTH_LOW; >> +tmp = DRIVE_STRENGTH_BIT_MSK(LOW); >> >> return tmp; >> } >> @@ -564,7 +569,7 @@ static unsigned at91_mux_sam9x5_get_drivestrength(void >> __iomem *pio, >> >> /* strength is inverse in SAM9x5s hardware with the pinctrl defines >> * hardware: 0 = hi, 1 = med, 2 = low, 3 = rsvd */ >> -tmp = DRIVE_STRENGTH_HI - tmp; >> +tmp = DRIVE_STRENGTH_BIT_MSK(HI) - tmp; >> >> return tmp; >> } >> @@ -600,7 +605,7 @@ static void at91_mux_sam9x5_set_drivestrength(void >> __iomem *pio, unsigned pin, >> >> /* strength is inverse on SAM9x5s with our defines >> * 0 = hi, 1 = med, 2 = low, 3 = rsvd */ >> -setting = DRIVE_STRENGTH_HI - setting; >> +setting = DRIVE_STRENGTH_BIT_MSK(HI) - setting; >> >> set_drive_strength(pio + at91sam9x5_get_drive_register(pin), pin, >> setting); >> @@ -959,11 +964,11 @@ static int at91_pinconf_set(struct pinctrl_dev >> *pctldev, >> } \ >> } while (0) >> >> -#define DBG_SHOW_FLAG_MASKED(mask,flag) do {\ >> +#define DBG_SHOW_FLAG_MASKED(mask,flag,name) do { \ > > checkpatch error: space required I'm aware of that but I added it in the way it was previously (were the checkpatch error was still present). Please let me know if you want me to send a new version fixing this. Thank you, Claudiu Beznea > >> if ((config & mask) == flag) { \ >> if (num_conf) \ >> seq_puts(s, "|"); \ >> -seq_puts(s, #flag); \ >> +seq_puts(s, #name); \ >> num_conf++; \ >> } \ >> } while (0) >> @@ -981,9 +986,12 @@ static void at91_pinconf_dbg_show(struct pinctrl_dev >> *pctldev, >> DBG_SHOW_FLAG(PULL_DOWN); >> DBG_SHOW_FLAG(DIS_SCHMIT); >> DBG_SHOW_FLAG(DEGLITCH); >> -DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_LOW); >> -DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_MED); >> -DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_HI); >> +DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_BIT_MSK(LOW), >> + DRIVE_STRENGTH_LOW); >> +DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_BIT_MSK(MED), >> + DRIVE_STRENGTH_MED); >> +DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_BIT_MSK(HI), >> + DRIVE_STRENGTH_HI); >> DBG_SHOW_FLAG(DEBOUNCE); >> if (config & DEBOUNCE) { >> val = config >> DEBOUNCE_VAL_SHIFT; >> -- >> 2.7.4 >> >
Re: [BUG BISECT] NULL pointer after commit "ASoC: dapm: Only power up active channels from a DAI"
Hi Krzysztof, Cc: Charles On 2/5/19 22:16, Krzysztof Kozlowski wrote: > Bisect pointed to commit: > commit 078a85f2806f0ffd11289009462a6a390f9adb5c > Author: Charles Keepax > Date: Thu Jan 31 13:30:18 2019 + > ASoC: dapm: Only power up active channels from a DAI > > as a bad commit for NULL pointer on my Odroid XU3 and Odroid U3 board when > doing "aplay /usr/share/sounds/alsa/Front_Right.wav". > > 1. Arch ARM Linux > 2. exynos_defconfig > 3. Odroid U3, XU3, Exynos SoC, ARMv7 > > Last address is in calltrace: > c079552c > dapm_update_dai_unlocked > sound/soc/soc-dapm.c:2586 Thanks for bisecting this, I ran into same issue last night and I'm starting to debug this now. I have added some debug prints and it looks like it oopses on NULL playback_widget of the dummy DAI. [ 30.701182] hdmi-audio-codec hdmi-audio-codec.0.auto: Update DAI routes for i2s-hifi playback [ 30.709630] dapm_update_dai_unlocked:2586 w=8bd27d28 [ 30.714403] dapm_update_dai_unlocked:2594 w=8bd27d28 [ 30.724688] max98090 5-0010: Update DAI routes for HiFi playback [ 30.730163] dapm_update_dai_unlocked:2586 w=3fc942af [ 30.735154] dapm_update_dai_unlocked:2594 w=3fc942af [ 30.745051] snd-soc-dummy snd-soc-dummy: Update DAI routes for snd-soc-dummy-dai playback [ 30.753128] dapm_update_dai_unlocked:2586 w= (null) [ 30.758114] Unable to handle kernel NULL pointer dereference at virtual address 007c -- Regards, Sylwester
Re: [LSF/MM TOPIC] Discuss least bad options for resolving longterm-GUP usage by RDMA
On Tue 05-02-19 09:50:59, Ira Weiny wrote: > The problem: Once we have pages marked as GUP-pinned how should various > subsystems work with those markings. > > The current work for John Hubbards proposed solutions (part 1 and 2) is > progressing.[1] But the final part (3) of his solution is also going to take > some work. > > In Johns presentation he lists 3 alternatives for gup-pinned pages: > > 1) Hold off try_to_unmap > 2) Allow writeback while pinned (via bounce buffers) > [Note this will not work for DAX] Well, but DAX does not need it because by definition there's nothing to writeback :) > 3) Use a "revocable reservation" (or lease) on those pages > 4) Pin the blocks as busy in the FS allocator > > The problem with lease's on pages used by RDMA is that the references to > these pages is not local to the machine. Once the user has been given > access to the page they, through the use of a remote tokens, give a > reference to that page to remote nodes. This is the core essence of > RDMA, and like it or not, something which is increasingly used by major > Linux users. > > Therefore we need to discuss the extent by which leases are appropriate and > what happens should a lease be revoked which a user does not respond to. I don't know the RDMA hardware so this is just an opinion of filesystem / mm guy but my idea how this should work would be: MM/FS asks for lease to be revoked. The revoke handler agrees with the other side on cancelling RDMA or whatever and drops the page pins. Now I understand there can be HW / communication failures etc. in which case the driver could either block waiting or make sure future IO will fail and drop the pins. But under normal conditions there should be a way to revoke the access. And if the HW/driver cannot support this, then don't let it anywhere near DAX filesystem. Honza -- Jan Kara SUSE Labs, CR
[PATCH] rtc: 88pm860x: fix unintended sign extension
From: Colin Ian King Shifting a u8 by 24 will cause the value to be promoted to an integer. If the top bit of the u8 is set then the following conversion to an unsigned long will sign extend the value causing the upper 32 bits to be set in the result. Fix this by casting the u8 value to an unsigned long before the shift. Detected by CoverityScan, CID#144925-144928 ("Unintended sign extension") Fixes: 008b30408c40 ("mfd: Add rtc support to 88pm860x") Signed-off-by: Colin Ian King --- drivers/rtc/rtc-88pm860x.c | 21 ++--- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/drivers/rtc/rtc-88pm860x.c b/drivers/rtc/rtc-88pm860x.c index 01ffc0ef8033..d25282b4a7dd 100644 --- a/drivers/rtc/rtc-88pm860x.c +++ b/drivers/rtc/rtc-88pm860x.c @@ -115,11 +115,13 @@ static int pm860x_rtc_read_time(struct device *dev, struct rtc_time *tm) pm860x_page_bulk_read(info->i2c, REG0_ADDR, 8, buf); dev_dbg(info->dev, "%x-%x-%x-%x-%x-%x-%x-%x\n", buf[0], buf[1], buf[2], buf[3], buf[4], buf[5], buf[6], buf[7]); - base = (buf[1] << 24) | (buf[3] << 16) | (buf[5] << 8) | buf[7]; + base = ((unsigned long)buf[1] << 24) | (buf[3] << 16) | + (buf[5] << 8) | buf[7]; /* load 32-bit read-only counter */ pm860x_bulk_read(info->i2c, PM8607_RTC_COUNTER1, 4, buf); - data = (buf[3] << 24) | (buf[2] << 16) | (buf[1] << 8) | buf[0]; + data = ((unsigned long)buf[3] << 24) | (buf[2] << 16) | + (buf[1] << 8) | buf[0]; ticks = base + data; dev_dbg(info->dev, "get base:0x%lx, RO count:0x%lx, ticks:0x%lx\n", base, data, ticks); @@ -145,7 +147,8 @@ static int pm860x_rtc_set_time(struct device *dev, struct rtc_time *tm) /* load 32-bit read-only counter */ pm860x_bulk_read(info->i2c, PM8607_RTC_COUNTER1, 4, buf); - data = (buf[3] << 24) | (buf[2] << 16) | (buf[1] << 8) | buf[0]; + data = ((unsigned long)buf[3] << 24) | (buf[2] << 16) | + (buf[1] << 8) | buf[0]; base = ticks - data; dev_dbg(info->dev, "set base:0x%lx, RO count:0x%lx, ticks:0x%lx\n", base, data, ticks); @@ -170,10 +173,12 @@ static int pm860x_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm) pm860x_page_bulk_read(info->i2c, REG0_ADDR, 8, buf); dev_dbg(info->dev, "%x-%x-%x-%x-%x-%x-%x-%x\n", buf[0], buf[1], buf[2], buf[3], buf[4], buf[5], buf[6], buf[7]); - base = (buf[1] << 24) | (buf[3] << 16) | (buf[5] << 8) | buf[7]; + base = ((unsigned long)buf[1] << 24) | (buf[3] << 16) | + (buf[5] << 8) | buf[7]; pm860x_bulk_read(info->i2c, PM8607_RTC_EXPIRE1, 4, buf); - data = (buf[3] << 24) | (buf[2] << 16) | (buf[1] << 8) | buf[0]; + data = ((unsigned long)buf[3] << 24) | (buf[2] << 16) | + (buf[1] << 8) | buf[0]; ticks = base + data; dev_dbg(info->dev, "get base:0x%lx, RO count:0x%lx, ticks:0x%lx\n", base, data, ticks); @@ -198,11 +203,13 @@ static int pm860x_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm) pm860x_page_bulk_read(info->i2c, REG0_ADDR, 8, buf); dev_dbg(info->dev, "%x-%x-%x-%x-%x-%x-%x-%x\n", buf[0], buf[1], buf[2], buf[3], buf[4], buf[5], buf[6], buf[7]); - base = (buf[1] << 24) | (buf[3] << 16) | (buf[5] << 8) | buf[7]; + base = ((unsigned long)buf[1] << 24) | (buf[3] << 16) | + (buf[5] << 8) | buf[7]; /* load 32-bit read-only counter */ pm860x_bulk_read(info->i2c, PM8607_RTC_COUNTER1, 4, buf); - data = (buf[3] << 24) | (buf[2] << 16) | (buf[1] << 8) | buf[0]; + data = ((unsigned long)buf[3] << 24) | (buf[2] << 16) | + (buf[1] << 8) | buf[0]; ticks = base + data; dev_dbg(info->dev, "get base:0x%lx, RO count:0x%lx, ticks:0x%lx\n", base, data, ticks); -- 2.20.1
Re: [PATCH v2 0/9] driver core: Fix some device links issues and add "consumer autoprobe" flag
On Tue, Feb 5, 2019 at 12:27 PM Rafael J. Wysocki wrote: > > On Tuesday, February 5, 2019 9:15:49 AM CET Ulf Hansson wrote: > > On Mon, 4 Feb 2019 at 12:45, Rafael J. Wysocki wrote: > > > > > > On Mon, Feb 4, 2019 at 12:40 PM Rafael J. Wysocki > > > wrote: > > > > > > > > On Fri, Feb 1, 2019 at 4:18 PM Ulf Hansson > > > > wrote: [cut] > > > > > > For example, if the consumer device is suspended after the > > > device_link_add() that incremented the supplier's PM-runtime count and > > > then resumed again, the rpm_active refcount will be greater than one > > > because of the last resume and not because of the initial link > > > creation. In that case, dropping the supplier's PM-runtime count on > > > link deletion may not work as expected. > > > > I see what your are saying and I must admit, by looking at the code, > > that it has turned into being rather complicated. Assuming of good > > reasons, of course. > > > > Anyway, I will play a little bit more with my tests to see what I can find > > out. > > > > > > > > > Arguably, device_link_del() could be made automatically drop the > > > > supplier's PM-runtime count by one if the link's rpm_active refcount > > > > is not one, but there will be failing scenarios in that case too > > > > AFAICS. > > > > Let's see. > > So for the record, below is the (untested) patch I'm thinking about. > > Having considered this for some time, I think that it would be better to > try to drop the supplier's PM-runtime usage counter on link removal even if > the link doesn't go away then. That would be more consistent at least IMO. So I can't convince myself that this is the case. Either way, if there are two callers of device_link_add() for one consumer-supplier pair trying to add a stateless link between them and one of these callers passes DL_FLAG_RPM_ACTIVE set in the flags to it, there may be issues regardless of what device_link_del() and device_link_remove() do. However, if they decrement the link's rpm_active refcount (and possibly the supplier's PM-runtime usage counter too), the supplier may be suspended prematurely, whereas in the other case (no decrementation of rpm_active, which how the code works after this series) it may just be prevented from suspending. To me, the former is worse than the latter. Moreover, there is a workaround for the latter issue that seems to be easy enough: it is sufficient to let the consumer runtime suspend after calling device_link_add() to create the link (with DL_FLAG_RPM_ACTIVE set) and before trying to remove it. Because of the above, I'm just going to post a patch to document the current behavior of the code as a known limitation.
[PATCH 1/2] PCI: dwc: allow to limit registers set length
Add length to the struct dw_pcie and check that the accessors dw_pcie_(rd|wr)_conf() do not read/write beyond that point. Suggested-by: Trent Piepho Signed-off-by: Stefan Agner --- Changes in v4: - Move length check to dw_pcie_rd_conf Changes in v5: - Rebased ontop of pci/dwc .../pci/controller/dwc/pcie-designware-host.c| 16 ++-- drivers/pci/controller/dwc/pcie-designware.h | 1 + 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c index 45ff5e4f8af6..bad54204fb52 100644 --- a/drivers/pci/controller/dwc/pcie-designware-host.c +++ b/drivers/pci/controller/dwc/pcie-designware-host.c @@ -612,14 +612,20 @@ static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where, int size, u32 *val) { struct pcie_port *pp = bus->sysdata; + struct dw_pcie *pci; if (!dw_pcie_valid_device(pp, bus, PCI_SLOT(devfn))) { *val = 0x; return PCIBIOS_DEVICE_NOT_FOUND; } - if (bus->number == pp->root_bus_nr) + if (bus->number == pp->root_bus_nr) { + pci = to_dw_pcie_from_pp(pp); + if (pci->dbi_length && where + size > pci->dbi_length) + return PCIBIOS_BAD_REGISTER_NUMBER; + return dw_pcie_rd_own_conf(pp, where, size, val); + } return dw_pcie_rd_other_conf(pp, bus, devfn, where, size, val); } @@ -628,12 +634,18 @@ static int dw_pcie_wr_conf(struct pci_bus *bus, u32 devfn, int where, int size, u32 val) { struct pcie_port *pp = bus->sysdata; + struct dw_pcie *pci; if (!dw_pcie_valid_device(pp, bus, PCI_SLOT(devfn))) return PCIBIOS_DEVICE_NOT_FOUND; - if (bus->number == pp->root_bus_nr) + if (bus->number == pp->root_bus_nr) { + pci = to_dw_pcie_from_pp(pp); + if (pci->dbi_length && where + size > pci->dbi_length) + return PCIBIOS_BAD_REGISTER_NUMBER; + return dw_pcie_wr_own_conf(pp, where, size, val); + } return dw_pcie_wr_other_conf(pp, bus, devfn, where, size, val); } diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h index 279000255ad1..d1d95119a422 100644 --- a/drivers/pci/controller/dwc/pcie-designware.h +++ b/drivers/pci/controller/dwc/pcie-designware.h @@ -226,6 +226,7 @@ struct dw_pcie_ops { struct dw_pcie { struct device *dev; void __iomem*dbi_base; + int dbi_length; void __iomem*dbi_base2; /* Used when iatu_unroll_enabled is true */ void __iomem*atu_base; -- 2.20.1
[PATCH 2/2] PCI: imx6: limit DBI register length
Define the length of the DBI registers. This makes sure that the kernel does not access registers beyond that point, avoiding the following abort on a i.MX 6Quad: # cat /sys/devices/soc0/soc/1ffc000.pcie/pci\:00/\:00\:00.0/config [ 100.021433] Unhandled fault: imprecise external abort (0x1406) at 0xb6ea7000 ... [ 100.056423] PC is at dw_pcie_read+0x50/0x84 [ 100.060790] LR is at dw_pcie_rd_own_conf+0x44/0x48 ... Signed-off-by: Stefan Agner Reviewed-by: Lucas Stach --- Changes in v3: - Rebase on pci/dwc Changes in v4: - Rebase on pci/dwc Changes in v5: - Rebased ontop of pci/dwc - Use DBI length of 0x200 drivers/pci/controller/dwc/pci-imx6.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c index c1d434ba3642..1ef7be1232f3 100644 --- a/drivers/pci/controller/dwc/pci-imx6.c +++ b/drivers/pci/controller/dwc/pci-imx6.c @@ -55,6 +55,7 @@ enum imx6_pcie_variants { struct imx6_pcie_drvdata { enum imx6_pcie_variants variant; u32 flags; + int dbi_length; }; struct imx6_pcie { @@ -1087,6 +1088,8 @@ static int imx6_pcie_probe(struct platform_device *pdev) break; } + pci->dbi_length = imx6_pcie->drvdata->dbi_length; + /* Grab turnoff reset */ imx6_pcie->turnoff_reset = devm_reset_control_get_optional_exclusive(dev, "turnoff"); if (IS_ERR(imx6_pcie->turnoff_reset)) { @@ -1170,6 +1173,7 @@ static const struct imx6_pcie_drvdata drvdata[] = { .variant = IMX6Q, .flags = IMX6_PCIE_FLAG_IMX6_PHY | IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE, + .dbi_length = 0x200, }, [IMX6SX] = { .variant = IMX6SX, -- 2.20.1
Re: [PATCH] rtc: rs5c372: Fix reading from rtc when the oscillator got interrupted.
On 06/02/2019 06:33:39+, oliver.r...@wago.com wrote: > >> + switch (rs5c->type) { > >> + case rtc_r2025sd: > >> + case rtc_r2221tl: > >> + if (ctrl2 & R2x2x_CTRL2_VDET) > >> + dev_warn(&client->dev, "rtc battery voltage drop below > >> threshold detected.\n"); > > > > VDET doesn't mean anything specific regarding timekeeping so I wouldn't > > warn here. > > VDET means in case the chip is powered by a battery, that is about to die > sometime soon, it gives you heads up. Yes but my statement stands true, this doesn't have any direct implication regarding timekeeping. There is the RTC_VL_READ ioctl to check voltage low. It is not weel designed but this is what we have now. > > > >> + if (ctrl2 & R2x2x_CTRL2_PON) > >> + dev_warn(&client->dev, "rtc battery voltage drop to > >> zero detected.\n"); > > > > You should return -EINVAL directly here > > The PON implies the XSTP bit, so I only return from there, just to print all > the warning messages, but I can return directly from here if you want. As you said, PON implies XSTP so I'm not sure it is valuable to have both warnings. -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [BUG BISECT] NULL pointer after commit "ASoC: dapm: Only power up active channels from a DAI"
On Tue, Feb 05, 2019 at 10:16:22PM +0100, Krzysztof Kozlowski wrote: > Bisect pointed to commit: > commit 078a85f2806f0ffd11289009462a6a390f9adb5c > Author: Charles Keepax > Date: Thu Jan 31 13:30:18 2019 + > ASoC: dapm: Only power up active channels from a DAI > > as a bad commit for NULL pointer on my Odroid XU3 and Odroid U3 board when > doing "aplay /usr/share/sounds/alsa/Front_Right.wav". > > 1. Arch ARM Linux > 2. exynos_defconfig > 3. Odroid U3, XU3, Exynos SoC, ARMv7 > > Last address is in calltrace: > c079552c > dapm_update_dai_unlocked > sound/soc/soc-dapm.c:2586 > Apologies for this one, is a little strange. I am assuming either we end up with a NULL widget or more likely given the line number a NULL path/widget on the path. Having some difficulty seeing how we get into that state though. Could you confirm what machine driver/DT is being used here? I am assuming this is arch/arm/boot/dts/exynos4412-odroidu3.dts and sound/soc/samsung/odroid.c. So will investigate those see if I can find anything. And could you try this diff and send through the output: diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index 482ddb825fb59..9ee44b0192874 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -2570,16 +2570,37 @@ static int dapm_update_dai_unlocked(struct snd_pcm_substream *substream, else w = dai->capture_widget; + if (!w) { + pr_err("CK -- NULL widget on %s\n", dai->name); + return 0; + } + dev_dbg(dai->dev, "Update DAI routes for %s %s\n", dai->name, dir == SNDRV_PCM_STREAM_PLAYBACK ? "playback" : "capture"); snd_soc_dapm_widget_for_each_sink_path(w, p) { + if (!p) { + pr_err("DEBUG -- NULL sink path on %s\n", w->name); + continue; + } + if (!p->sink) { + pr_err("DEBUG -- NULL wink widget on %s\n", w->name); + continue; + } ret = dapm_update_dai_chan(p, p->sink, channels); if (ret < 0) return ret; } snd_soc_dapm_widget_for_each_source_path(w, p) { + if (!p) { + pr_err("DEBUG -- NULL source path on %s\n", w->name); + continue; + } + if (!p->source) { + pr_err("DEBUG -- NULL source widget on %s\n", w->name); + continue; + } ret = dapm_update_dai_chan(p, p->source, channels); if (ret < 0) return ret; Thanks, Charles
Re: [PATCH 3/9] mfd: pm8xxx: convert to v2 irq interfaces to support hierarchical IRQ chips
Hi Brian! Thanks for this patch! On Fri, Jan 25, 2019 at 5:23 PM Brian Masney wrote: > The IRQ handler was hardcoded as handle_level_irq and this patch > properly sets the handler to either handle_edge_irq or handle_level_irq > depending on the IRQ type. Done like so: > + flow_handler = handle_edge_irq; This creates a problem and makes my kernel crash, because handle_edge_irq() *unconditionally* calls chip->irq_ack(), and since this chip does not implement .irq_ack(), it crashes. I tried to just add a void .irq_ack(). That instead made the kernel hang. If I just assign handle_level_irq() as before (just replaced all assignments of handle_edge_irq with handle_level_irq) it boots nicely, AND the interrupts are working fine on the ADC calibration path, and further readings of the ADC generates new IRQs, as expected! The handle_edge_irq and .irq_ack() call path is for hardware that require an explicit ACK (such as writing a bit in some register) when handling edge IRQs. This is necessary in some hardware because of the transient nature of edge IRQs. This is not necessary on some hardware that will simply clear the flag when something is reading the status register. I think this is the case with PM8xxx and you can safely use handle_level_irq with all IRQs. It looks counterintuitive and maybe the function handle_edge_irq() has a stupid name. Just drop this handle_edge_irq() business for now. This was tested on the APQ8060 DragonBoard. However this happens: [3.007727] mmci-pl18x 1218.sdcc: ignoring dependency for device, assuming no driver [3.007970] mmci-pl18x 1220.sdcc: ignoring dependency for device, assuming no driver [3.014984] mpu3050-i2c 1-0068: ignoring dependency for device, assuming no driver [3.023543] cm3605 cm3605: ignoring dependency for device, assuming no driver [3.030532] ak8975 1-000c: ignoring dependency for device, assuming no driver [3.037729] bmp280 1-0077: ignoring dependency for device, assuming no driver [3.044789] reg-fixed-voltage regulators:xc622a331mrg: ignoring dependency for device, assuming no driver [3.052485] smsc911x 1b80.ethernet-ebi2: ignoring dependency for device, assuming no driver But I think that is because these all use ssbi-gpio interrupts and these are now handled properly so I will try to fix the device tree and send a separate patch that you can include. Yours, Linus Walleij
Re: [RFC PATCH] clk: sunxi-ng: sun4i: Use CLK_SET_RATE_PARENT for mmc2 clock
On Wed, Feb 06, 2019 at 10:20:00AM +0100, Maxime Ripard wrote: > On Tue, Feb 05, 2019 at 09:44:02PM +0800, Chen-Yu Tsai wrote: > > On Tue, Feb 5, 2019 at 5:45 PM Maxime Ripard > > wrote: > > > > > > On Sat, Feb 02, 2019 at 05:52:09PM +0200, Priit Laes wrote: > > > > Recent patch of improving MP clock rate calculations by taking > > > > into account whether adjusting parent rate is allowed, have > > > > unfortunately broken eMMC support on A20 Olinuxino-Lime2-eMMC > > > > boards which fail with following error: > > > > > > > > [snip] > > > > EXT4-fs (mmcblk1p4): INFO: recovery required on readonly filesystem > > > > EXT4-fs (mmcblk1p4): write access will be enabled during recovery > > > > sunxi-mmc 1c11000.mmc: data error, sending stop command > > > > sunxi-mmc 1c11000.mmc: send stop command failed > > > > [/snip] > > > > > > > > Previously, mmc2 clock was requesting 520MHz and settling at 512MHz > > > > clock rate with following parents: > > > > You mean 52 and 51.2 MHz. > > > > > > [snip] > > > > pll-ddr-base2 20 76800 0 0 5 > > > >pll-ddr-other1 10 76800 0 0 5 > > > > mmc2 0 005120 0 0 5 > > > > [/snip] > > > > > > > > Now, after the improvements, requested and settled rate are both > > > > 520MHz, but as mmc2 clock cannot adjust parent rate, the situation > > > > ends up like this: > > > > [snip] > > > > pll-periph-base 3 30 12 0 0 5 > > > >pll-periph 6 60 6 0 0 5 > > > > mmc2 3 305000 0 0 5 > > > > [/snip] > > > > > > > > With this patch (allowing mmc2 to set parent rate), we end up with > > > > working tree with both mmc0 (sd-card) and mmc2 (eMMC) working: > > > > [snip] > > > > pll-periph-base 3 30 31200 0 0 5 > > > >mbus 1 107800 0 0 5 > > > >pll-periph-sata 1 102600 0 0 5 > > > > sata 1 102600 0 0 5 > > > >pll-periph 5 50 15600 0 0 5 > > > > mmc2 0 005200 0 0 5 > > > > mmc0 0 003900 0 0 5 > > > > [/snip] > > > > > > > > Fixes: 3f790433c3cb ("clk: sunxi-ng: Adjust MP clock parent rate when > > > > allowed") > > > > Signed-off-by: Priit Laes > > > > > > Applied, thanks! > > > Maxime > > > > I'm concerned for other users of the PLL-PERIPH clock. AFAIK > > all of them, except the HRTIMER, expect the clock rate to stay > > the same and not change underneath them. And SATA expects it to > > be at 600 MHz, as the datasheet says. And while it may not directly > > apply to the LIME2, eMMC on newer SoCs / boards run at the slightly > > reduced rate of 50 MHz just fine. > > > > In the commit in question, clocks without CLK_SET_RATE_PARENT > > should be using the old code (now in the if conditional block), > > i.e. the behavior should not have changed. > > > > I don't think this actually "fixes" whatever bug was introduced, > > but only papers over the issue, and possible introduces further > > issues for other users. > > You're right, I've overlooked that it was pll-periph being > affected. I've dropped it for now. Any ideas what could be done. I currently have no time to debug it, but it affects existing systems. > > Maxime > > -- > Maxime Ripard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
Re: Build regressions/improvements in v5.0-rc5
Randy Dunlap writes: > On 2/4/19 9:46 AM, Randy Dunlap wrote: >> On 2/4/19 5:42 AM, Geert Uytterhoeven wrote: >>> Below is the list of build error/warning regressions/improvements in >>> v5.0-rc5[1] compared to v4.20[2]. >>> >>> Summarized: >>> - build errors: +2/-4 >>> - build warnings: +113/-14843 >>> >>> JFYI, when comparing v5.0-rc5[1] to v5.0-rc4[3], the summaries are: >>> - build errors: +0/-0 >>> - build warnings: +56/-59 >>> >>> Note that there may be false regressions, as some logs are incomplete. >>> Still, they're build errors/warnings. >>> >>> Happy fixing! ;-) >>> >>> Thanks to the linux-next team for providing the build service. >>> >>> [1] >>> http://kisskb.ellerman.id.au/kisskb/branch/linus/head/8834f5600cf3c8db365e18a3d5cac2c2780c81e5/ >>> (238 out of 240 configs) >>> [2] >>> http://kisskb.ellerman.id.au/kisskb/branch/linus/head/8fe28cb58bcb235034b64cbbb7550a8a43fd88be/ >>> (all 240 configs) >>> [3] >>> http://kisskb.ellerman.id.au/kisskb/branch/linus/head/f17b5f06cb92ef2250513a1e154c47b78df07d40/ >>> (238 out of 240 configs) >>> >>> >>> >>> *** WARNINGS *** >>> >>> 113 warning regressions: >> >>> + warning: unmet direct dependencies detected for PPC4xx_PCI_EXPRESS: => >>> N/A >> >> Hi Geert, >> >> I am trying to find the above warning (needle) in the haystack. >> Can you please direct me to which config/build it is in? >> >> Is there a good way for me to find it? >> >> thanks. >> > > Working backwards, it probably comes from this: > > config CURRITUCK > bool "IBM Currituck (476fpe) Support" > depends on PPC_47x > select SWIOTLB > select 476FPE > select PPC4xx_PCI_EXPRESS > > This is the only similar Kconfig segment that does not select FORCE_PCI > but PPC4xx_PCI_EXPRESS depends on PCI but > # CONFIG_PCI is not set > > This could be related to a recent series of patches that allows ACPI to > be built without having PCI enabled. > > Michael? There isn't really an easy way to do it sorry. If you download the page for the revision: http://kisskb.ellerman.id.au/kisskb/branch/linus/head/8834f5600cf3c8db365e18a3d5cac2c2780c81e5/ With a lot of grep/sed/awk you can get a list of the URLs for all the builds eg. one would be: http://kisskb.ellerman.id.au/kisskb/buildresult/13676551/ And then you can just tack on "log/" to get: http://kisskb.ellerman.id.au/kisskb/buildresult/13676551/log/ Which you can wget and grep. But that's all a bit of a pain. I have on my todo list to add json formatted build results and revision pages, which would make it quite a bit simpler. But of course I haven't had time to do it yet :/ cheers
[PATCH] ASoC: dapm: Check for NULL widget in dapm_update_dai_unlocked
DAIs linked to the dummy will not have an associated playback/capture widget, so we need to skip the update in that case. Fixes: 078a85f2806f ("ASoC: dapm: Only power up active channels from a DAI") Signed-off-by: Charles Keepax --- Ok so that all makes sense, this patch is probably the best fix? Thanks, Charles sound/soc/soc-dapm.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index 482ddb825fb59..5235d8828758a 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -2570,6 +2570,9 @@ static int dapm_update_dai_unlocked(struct snd_pcm_substream *substream, else w = dai->capture_widget; + if (!w) + return 0; + dev_dbg(dai->dev, "Update DAI routes for %s %s\n", dai->name, dir == SNDRV_PCM_STREAM_PLAYBACK ? "playback" : "capture"); -- 2.11.0
[PATCH] rtc: 88pm80x: fix unintended sign extension
From: Colin Ian King Shifting a u8 by 24 will cause the value to be promoted to an integer. If the top bit of the u8 is set then the following conversion to an unsigned long will sign extend the value causing the upper 32 bits to be set in the result. Fix this by casting the u8 value to an unsigned long before the shift. Detected by CoverityScan, CID#714646-714649 ("Unintended sign extension") Fixes: 2985c29c1964 ("rtc: Add rtc support to 88PM80X PMIC") Signed-off-by: Colin Ian King --- drivers/rtc/rtc-88pm80x.c | 21 ++--- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/drivers/rtc/rtc-88pm80x.c b/drivers/rtc/rtc-88pm80x.c index cab293cb2bf0..1fc48ebd3cd0 100644 --- a/drivers/rtc/rtc-88pm80x.c +++ b/drivers/rtc/rtc-88pm80x.c @@ -114,12 +114,14 @@ static int pm80x_rtc_read_time(struct device *dev, struct rtc_time *tm) unsigned char buf[4]; unsigned long ticks, base, data; regmap_raw_read(info->map, PM800_RTC_EXPIRE2_1, buf, 4); - base = (buf[3] << 24) | (buf[2] << 16) | (buf[1] << 8) | buf[0]; + base = ((unsigned long)buf[3] << 24) | (buf[2] << 16) | + (buf[1] << 8) | buf[0]; dev_dbg(info->dev, "%x-%x-%x-%x\n", buf[0], buf[1], buf[2], buf[3]); /* load 32-bit read-only counter */ regmap_raw_read(info->map, PM800_RTC_COUNTER1, buf, 4); - data = (buf[3] << 24) | (buf[2] << 16) | (buf[1] << 8) | buf[0]; + data = ((unsigned long)buf[3] << 24) | (buf[2] << 16) | + (buf[1] << 8) | buf[0]; ticks = base + data; dev_dbg(info->dev, "get base:0x%lx, RO count:0x%lx, ticks:0x%lx\n", base, data, ticks); @@ -137,7 +139,8 @@ static int pm80x_rtc_set_time(struct device *dev, struct rtc_time *tm) /* load 32-bit read-only counter */ regmap_raw_read(info->map, PM800_RTC_COUNTER1, buf, 4); - data = (buf[3] << 24) | (buf[2] << 16) | (buf[1] << 8) | buf[0]; + data = ((unsigned long)buf[3] << 24) | (buf[2] << 16) | + (buf[1] << 8) | buf[0]; base = ticks - data; dev_dbg(info->dev, "set base:0x%lx, RO count:0x%lx, ticks:0x%lx\n", base, data, ticks); @@ -158,11 +161,13 @@ static int pm80x_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm) int ret; regmap_raw_read(info->map, PM800_RTC_EXPIRE2_1, buf, 4); - base = (buf[3] << 24) | (buf[2] << 16) | (buf[1] << 8) | buf[0]; + base = ((unsigned long)buf[3] << 24) | (buf[2] << 16) | + (buf[1] << 8) | buf[0]; dev_dbg(info->dev, "%x-%x-%x-%x\n", buf[0], buf[1], buf[2], buf[3]); regmap_raw_read(info->map, PM800_RTC_EXPIRE1_1, buf, 4); - data = (buf[3] << 24) | (buf[2] << 16) | (buf[1] << 8) | buf[0]; + data = ((unsigned long)buf[3] << 24) | (buf[2] << 16) | + (buf[1] << 8) | buf[0]; ticks = base + data; dev_dbg(info->dev, "get base:0x%lx, RO count:0x%lx, ticks:0x%lx\n", base, data, ticks); @@ -185,12 +190,14 @@ static int pm80x_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm) regmap_update_bits(info->map, PM800_RTC_CONTROL, PM800_ALARM1_EN, 0); regmap_raw_read(info->map, PM800_RTC_EXPIRE2_1, buf, 4); - base = (buf[3] << 24) | (buf[2] << 16) | (buf[1] << 8) | buf[0]; + base = ((unsigned long)buf[3] << 24) | (buf[2] << 16) | + (buf[1] << 8) | buf[0]; dev_dbg(info->dev, "%x-%x-%x-%x\n", buf[0], buf[1], buf[2], buf[3]); /* load 32-bit read-only counter */ regmap_raw_read(info->map, PM800_RTC_COUNTER1, buf, 4); - data = (buf[3] << 24) | (buf[2] << 16) | (buf[1] << 8) | buf[0]; + data = ((unsigned long)buf[3] << 24) | (buf[2] << 16) | + (buf[1] << 8) | buf[0]; ticks = base + data; dev_dbg(info->dev, "get base:0x%lx, RO count:0x%lx, ticks:0x%lx\n", base, data, ticks); -- 2.20.1
Re: [RFC v2] usb: xhci: add Immediate Data Transfer support
Hi Felipe, thanks for the review! On Wed, 2019-02-06 at 08:35 +0200, Felipe Balbi wrote: > Hi, > > Nicolas Saenz Julienne writes: > > diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c > > index 40fa25c4d041..a4efbe62a1a3 100644 > > --- a/drivers/usb/host/xhci-ring.c > > +++ b/drivers/usb/host/xhci-ring.c > > @@ -3272,8 +3272,15 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t > > mem_flags, > > field |= TRB_IOC; > > more_trbs_coming = false; > > td->last_trb = ring->enqueue; > > + > > + if (xhci_urb_suitable_for_idt(urb)) { > > + memcpy(&send_addr, urb->transfer_buffer, > > + trb_buff_len); > > + field |= TRB_IDT; > > + } > > } > > > > + > > trailing change Noted > > > @@ -3411,6 +3418,12 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t > > mem_flags, > > if (urb->transfer_buffer_length > 0) { > > u32 length_field, remainder; > > > > + if (xhci_urb_suitable_for_idt(urb)) { > > + memcpy(&urb->transfer_dma, urb->transfer_buffer, > > + urb->transfer_buffer_length); > > + field |= TRB_IDT; > > + } > > + > > remainder = xhci_td_remainder(xhci, 0, > > urb->transfer_buffer_length, > > urb->transfer_buffer_length, > > @@ -3420,6 +3433,7 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t > > mem_flags, > > TRB_INTR_TARGET(0); > > if (setup->bRequestType & USB_DIR_IN) > > field |= TRB_DIR_IN; > > + > > trailing change Noted > > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > > index 005e65922608..dec62f7f5dc8 100644 > > --- a/drivers/usb/host/xhci.c > > +++ b/drivers/usb/host/xhci.c > > @@ -1238,6 +1238,21 @@ EXPORT_SYMBOL_GPL(xhci_resume); > > > > /*- > > */ > > > > +/* > > + * Bypass the DMA mapping if URB is suitable for Immediate Transfer (IDT), > > + * we'll copy the actual data into the TRB address register. This is > > limited to > > + * transfers up to 8 bytes on output endpoints of any kind with > > wMaxPacketSize > > + * >= 8 bytes. > > + */ > > +static int xhci_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb, > > + gfp_t mem_flags) > > +{ > > + if (xhci_urb_suitable_for_idt(urb)) > > + return 0; > > + > > + return usb_hcd_map_urb_for_dma(hcd, urb, mem_flags); > > +} > > don't you need a matching unmap_urb_for_dma()?? Not really as every DMA mapping sets a matching URB flag to track it. For example when usb_hcd_map_urb_for_dma() uses dma_map_single() it will set URB_DMA_MAP_SINGLE in urb->transfer_flags, later on unmap_urb_for_dma() will catch it and unmap it. As I bypass the mapping altogether there are no flags set, so unmap_urb_for_dma() won't have any effect. I could still add it for clarity, and well, I guess it'll save some instructions on the IDT suitable side. > > > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h > > index 652dc36e3012..9d77b0901ab7 100644 > > --- a/drivers/usb/host/xhci.h > > +++ b/drivers/usb/host/xhci.h > > @@ -1295,6 +1295,8 @@ enum xhci_setup_dev { > > #define TRB_IOC(1<<5) > > /* The buffer pointer contains immediate data */ > > #define TRB_IDT(1<<6) > > +/* TDs smaller than this might use IDT */ > > Technically, "TDs at most this" since you're 8 itself is an allowed > size. > Noted Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [BUG BISECT] NULL pointer after commit "ASoC: dapm: Only power up active channels from a DAI"
On 2/6/19 10:46, Sylwester Nawrocki wrote: > On 2/5/19 22:16, Krzysztof Kozlowski wrote: >> Bisect pointed to commit: >> commit 078a85f2806f0ffd11289009462a6a390f9adb5c >> Author: Charles Keepax >> Date: Thu Jan 31 13:30:18 2019 + >> ASoC: dapm: Only power up active channels from a DAI >> >> as a bad commit for NULL pointer on my Odroid XU3 and Odroid U3 board when >> doing "aplay /usr/share/sounds/alsa/Front_Right.wav". >> >> 1. Arch ARM Linux >> 2. exynos_defconfig >> 3. Odroid U3, XU3, Exynos SoC, ARMv7 >> >> Last address is in calltrace: >> c079552c >> dapm_update_dai_unlocked >> sound/soc/soc-dapm.c:2586 > > Thanks for bisecting this, I ran into same issue last night and I'm starting > to debug this now. I have added some debug prints and it looks like it oopses > on NULL playback_widget of the dummy DAI. > > [ 30.701182] hdmi-audio-codec hdmi-audio-codec.0.auto: Update DAI routes > for i2s-hifi playback > [ 30.709630] dapm_update_dai_unlocked:2586 w=8bd27d28 > [ 30.714403] dapm_update_dai_unlocked:2594 w=8bd27d28 > [ 30.724688] max98090 5-0010: Update DAI routes for HiFi playback > [ 30.730163] dapm_update_dai_unlocked:2586 w=3fc942af > [ 30.735154] dapm_update_dai_unlocked:2594 w=3fc942af > [ 30.745051] snd-soc-dummy snd-soc-dummy: Update DAI routes for > snd-soc-dummy-dai playback > [ 30.753128] dapm_update_dai_unlocked:2586 w= (null) > [ 30.758114] Unable to handle kernel NULL pointer dereference at virtual > address 007c With a change as below there is no oops and everything works again, but I'm not sure this is a proper fix. diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index 5b74dffc9c11..111a23a9708a 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -2580,6 +2580,9 @@ static int dapm_update_dai_unlocked(struct snd_pcm_substream *substream, else w = dai->capture_widget; + if (!w) + return 0; + dev_dbg(dai->dev, "Update DAI routes for %s %s\n", dai->name, dir == SNDRV_PCM_STREAM_PLAYBACK ? "playback" : "capture"); -- Thanks, Sylwester
Re: [PATCH 2/2] cpufreq: mediatek: Register an Energy Model
Hi Matthias, On Tuesday 05 Feb 2019 at 09:52:25 (-0800), Matthias Kaehlcke wrote: > Try and register an Energy Model from mediatek-cpufreq to allow > interested subsystems like the task scheduler to use the provided > information. > > Signed-off-by: Matthias Kaehlcke > --- > drivers/cpufreq/mediatek-cpufreq.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/cpufreq/mediatek-cpufreq.c > b/drivers/cpufreq/mediatek-cpufreq.c > index eb8920d398181..e6168ee582783 100644 > --- a/drivers/cpufreq/mediatek-cpufreq.c > +++ b/drivers/cpufreq/mediatek-cpufreq.c > @@ -460,6 +460,8 @@ static int mtk_cpufreq_init(struct cpufreq_policy *policy) > return ret; > } > > + dev_pm_opp_of_register_em(policy->cpus); I'm not familiar with the mediatek-cpufreq driver so bear with me, but the code sets policy->cpus just below here. Is there any particular reason for not using that in PM_EM ? > cpumask_copy(policy->cpus, &info->cpus); > policy->freq_table = freq_table; > policy->driver_data = info; > -- > 2.20.1.611.gfbb209baf1-goog > Thanks, Quentin
[PATCH 6/8] net: thunderx: add mutex to protect mailbox from concurrent calls for same VF
In some cases it could happen that nicvf_send_msg_to_pf() could be called concurrently for the same NIC VF, and thus re-writing mailbox contents and breaking messaging sequence with PF by re-writing NICVF data. This commit is to implement mutex for NICVF to protect mailbox registers and NICVF messaging control data from concurrent access. Signed-off-by: Vadim Lomovtsev --- drivers/net/ethernet/cavium/thunder/nic.h| 2 ++ drivers/net/ethernet/cavium/thunder/nicvf_main.c | 13 ++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/cavium/thunder/nic.h b/drivers/net/ethernet/cavium/thunder/nic.h index 227343625e83..86cda3f4b37b 100644 --- a/drivers/net/ethernet/cavium/thunder/nic.h +++ b/drivers/net/ethernet/cavium/thunder/nic.h @@ -329,6 +329,8 @@ struct nicvf { spinlock_t rx_mode_wq_lock; /* workqueue for handling kernel ndo_set_rx_mode() calls */ struct workqueue_struct *nicvf_rx_mode_wq; + /* mutex to protect VF's mailbox contents from concurrent access */ + struct mutexrx_mode_mtx; /* PTP timestamp */ struct cavium_ptp *ptp_clock; diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c index 30c7f54b4f17..a05e2989ec76 100644 --- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c +++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c @@ -124,6 +124,9 @@ int nicvf_send_msg_to_pf(struct nicvf *nic, union nic_mbx *mbx) { int timeout = NIC_MBOX_MSG_TIMEOUT; int sleep = 10; + int ret = 0; + + mutex_lock(&nic->rx_mode_mtx); nic->pf_acked = false; nic->pf_nacked = false; @@ -136,7 +139,8 @@ int nicvf_send_msg_to_pf(struct nicvf *nic, union nic_mbx *mbx) netdev_err(nic->netdev, "PF NACK to mbox msg 0x%02x from VF%d\n", (mbx->msg.msg & 0xFF), nic->vf_id); - return -EINVAL; + ret = -EINVAL; + break; } msleep(sleep); if (nic->pf_acked) @@ -146,10 +150,12 @@ int nicvf_send_msg_to_pf(struct nicvf *nic, union nic_mbx *mbx) netdev_err(nic->netdev, "PF didn't ACK to mbox msg 0x%02x from VF%d\n", (mbx->msg.msg & 0xFF), nic->vf_id); - return -EBUSY; + ret = -EBUSY; + break; } } - return 0; + mutex_unlock(&nic->rx_mode_mtx); + return ret; } /* Checks if VF is able to comminicate with PF @@ -2211,6 +2217,7 @@ static int nicvf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) nic->vf_id); INIT_WORK(&nic->rx_mode_work.work, nicvf_set_rx_mode_task); spin_lock_init(&nic->rx_mode_wq_lock); + mutex_init(&nic->rx_mode_mtx); err = register_netdev(netdev); if (err) { -- 2.17.2
[PATCH 0/8] nic: thunderx: fix communication races betwen VF & PF
The ThunderX CN88XX NIC Virtual Function driver uses mailbox interface to communicate to physical function driver. Each of VF has it's own pair of mailbox registers to read from and write to. The mailbox registers has no protection from possible races, so it has to be implemented at software side. After long term testing by loop of 'ip link set up/down' command it was found that there are two possible scenarios when race condition appears: 1. VF receives link change message from PF and VF send RX mode configuration message to PF in the same time from separate thread. 2. PF receives RX mode configuration from VF and in the same time, in separate thread PF detects link status change and sends appropriate message to particular VF. Both cases leads to mailbox data to be rewritten, NIC VF messaging control data to be updated incorrectly and communication sequence gets broken. This patch series is to address race condition with VF & PF communication. Vadim Lomovtsev (8): net: thunderx: correct typo in macro name net: thunderx: replace global nicvf_rx_mode_wq work queue for all VFs to private for each of them. net: thunderx: make CFG_DONE message to run through generic send-ack sequence net: thunderx: add nicvf_send_msg_to_pf result check for set_rx_mode_task net: thunderx: rework xcast message structure to make it fit into 64 bit net: thunderx: add mutex to protect mailbox from concurrent calls for same VF net: thunderx: implement helpers to read mailbox IRQ status net: thunderx: check status of mailbox IRQ before sending a message drivers/net/ethernet/cavium/thunder/nic.h | 12 +-- .../net/ethernet/cavium/thunder/nic_main.c| 58 +++-- .../net/ethernet/cavium/thunder/nicvf_main.c | 82 +-- .../ethernet/cavium/thunder/nicvf_queues.c| 14 .../ethernet/cavium/thunder/nicvf_queues.h| 1 + .../net/ethernet/cavium/thunder/thunder_bgx.c | 2 +- .../net/ethernet/cavium/thunder/thunder_bgx.h | 2 +- 7 files changed, 112 insertions(+), 59 deletions(-) -- 2.17.2
[PATCH 8/8] net: thunderx: check status of mailbox IRQ before sending a message
In order to prevent mailbox data re-writing at VF side we need to check if there is an active mailbox IRQ from PF, and if there is no one proceed with sending message to PF. Having spinlock at irq handler and message send routing wont help since by the moment when code flow would reach the irq handler and acquire spinlock the message send routine could be already invoked and thus re-write data in the mailbox. The same is true for PF while sending messages to VF. This commit is to implement mailbox IRQ status check before sending message to VF from PF. Same is for sending message to PF from VF. Signed-off-by: Vadim Lomovtsev --- .../net/ethernet/cavium/thunder/nic_main.c| 39 --- .../net/ethernet/cavium/thunder/nicvf_main.c | 3 ++ 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/drivers/net/ethernet/cavium/thunder/nic_main.c b/drivers/net/ethernet/cavium/thunder/nic_main.c index a32c1bd75794..e0041692caef 100644 --- a/drivers/net/ethernet/cavium/thunder/nic_main.c +++ b/drivers/net/ethernet/cavium/thunder/nic_main.c @@ -64,7 +64,6 @@ struct nicpf { u32 *speed; u16 cpi_base[MAX_NUM_VFS_SUPPORTED]; u16 rssi_base[MAX_NUM_VFS_SUPPORTED]; - boolmbx_lock[MAX_NUM_VFS_SUPPORTED]; /* MSI-X */ u8 num_vec; @@ -954,8 +953,6 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf) int i; int ret = 0; - nic->mbx_lock[vf] = true; - mbx_addr = nic_get_mbx_addr(vf); mbx_data = (u64 *)&mbx; @@ -975,7 +972,7 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf) nic->duplex[vf] = 0; nic->speed[vf] = 0; } - goto unlock; + return; case NIC_MBOX_MSG_QS_CFG: reg_addr = NIC_PF_QSET_0_127_CFG | (mbx.qs.num << NIC_QS_ID_SHIFT); @@ -1044,7 +1041,7 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf) break; case NIC_MBOX_MSG_RSS_SIZE: nic_send_rss_size(nic, vf); - goto unlock; + return; case NIC_MBOX_MSG_RSS_CFG: case NIC_MBOX_MSG_RSS_CFG_CONT: nic_config_rss(nic, &mbx.rss_cfg); @@ -1062,19 +1059,19 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf) break; case NIC_MBOX_MSG_ALLOC_SQS: nic_alloc_sqs(nic, &mbx.sqs_alloc); - goto unlock; + return; case NIC_MBOX_MSG_NICVF_PTR: nic->nicvf[vf] = mbx.nicvf.nicvf; break; case NIC_MBOX_MSG_PNICVF_PTR: nic_send_pnicvf(nic, vf); - goto unlock; + return; case NIC_MBOX_MSG_SNICVF_PTR: nic_send_snicvf(nic, &mbx.nicvf); - goto unlock; + return; case NIC_MBOX_MSG_BGX_STATS: nic_get_bgx_stats(nic, &mbx.bgx_stats); - goto unlock; + return; case NIC_MBOX_MSG_LOOPBACK: ret = nic_config_loopback(nic, &mbx.lbk); break; @@ -1083,7 +1080,7 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf) break; case NIC_MBOX_MSG_PFC: nic_pause_frame(nic, vf, &mbx.pfc); - goto unlock; + return; case NIC_MBOX_MSG_PTP_CFG: nic_config_timestamp(nic, vf, &mbx.ptp); break; @@ -1134,8 +1131,6 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf) mbx.msg.msg, vf); nic_mbx_send_nack(nic, vf); } -unlock: - nic->mbx_lock[vf] = false; } static irqreturn_t nic_mbx_intr_handler(int irq, void *nic_irq) @@ -1313,18 +1308,18 @@ static void nic_poll_for_link(struct work_struct *work) if (nic->link[vf] == link.link_up) continue; - if (!nic->mbx_lock[vf]) { - nic->link[vf] = link.link_up; - nic->duplex[vf] = link.duplex; - nic->speed[vf] = link.speed; + nic->link[vf] = link.link_up; + nic->duplex[vf] = link.duplex; + nic->speed[vf] = link.speed; - /* Send a mbox message to VF with current link status */ - mbx.link_status.link_up = link.link_up; - mbx.link_status.duplex = link.duplex; - mbx.link_status.speed = link.speed; - mbx.link_status.mac_type = link.mac_type; + /* Send a mbox message to VF with current link status */ + mbx.link_status.link_up = link.link_up; + mbx.link_status.duplex = link.duplex; + mbx.link_status.speed = link.sp
[PATCH 1/8] net: thunderx: correct typo in macro name
Correct STREERING to STEERING at macro name for BGX steering register. Signed-off-by: Vadim Lomovtsev --- drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 2 +- drivers/net/ethernet/cavium/thunder/thunder_bgx.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c index e337da6ba2a4..673c57b8023f 100644 --- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c +++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c @@ -1217,7 +1217,7 @@ static void bgx_init_hw(struct bgx *bgx) /* Disable MAC steering (NCSI traffic) */ for (i = 0; i < RX_TRAFFIC_STEER_RULE_COUNT; i++) - bgx_reg_write(bgx, 0, BGX_CMR_RX_STREERING + (i * 8), 0x00); + bgx_reg_write(bgx, 0, BGX_CMR_RX_STEERING + (i * 8), 0x00); } static u8 bgx_get_lane2sds_cfg(struct bgx *bgx, struct lmac *lmac) diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.h b/drivers/net/ethernet/cavium/thunder/thunder_bgx.h index cbdd20b9ee6f..5cbc54e9eb19 100644 --- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.h +++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.h @@ -60,7 +60,7 @@ #define RX_DMACX_CAM_EN BIT_ULL(48) #define RX_DMACX_CAM_LMACID(x)(((u64)x) << 49) #define RX_DMAC_COUNT 32 -#define BGX_CMR_RX_STREERING 0x300 +#define BGX_CMR_RX_STEERING0x300 #define RX_TRAFFIC_STEER_RULE_COUNT 8 #define BGX_CMR_CHAN_MSK_AND 0x450 #define BGX_CMR_BIST_STATUS0x460 -- 2.17.2
[PATCH 7/8] net: thunderx: implement helpers to read mailbox IRQ status
This commit is to implement routines to read mailbox IRQ status for particular VF at PF side, and for mailbox IRQ status from PF at VF side. Signed-off-by: Vadim Lomovtsev --- drivers/net/ethernet/cavium/thunder/nic_main.c | 13 + drivers/net/ethernet/cavium/thunder/nicvf_queues.c | 14 ++ drivers/net/ethernet/cavium/thunder/nicvf_queues.h | 1 + 3 files changed, 28 insertions(+) diff --git a/drivers/net/ethernet/cavium/thunder/nic_main.c b/drivers/net/ethernet/cavium/thunder/nic_main.c index 620dbe082ca0..a32c1bd75794 100644 --- a/drivers/net/ethernet/cavium/thunder/nic_main.c +++ b/drivers/net/ethernet/cavium/thunder/nic_main.c @@ -104,6 +104,19 @@ static u64 nic_reg_read(struct nicpf *nic, u64 offset) return readq_relaxed(nic->reg_base + offset); } +static int nic_is_mbox_intr_active(struct nicpf *nic, int vf_id) +{ + int ret = 0; + + if (vf_id < NIC_VF_PER_MBX_REG) { + ret = nic_reg_read(nic, NIC_PF_MAILBOX_INT) & BIT_ULL(vf_id); + } else { + ret = nic_reg_read(nic, NIC_PF_MAILBOX_INT + sizeof(u64)) & + BIT_ULL(vf_id - NIC_VF_PER_MBX_REG); + } + return ret; +} + /* PF -> VF mailbox communication APIs */ static void nic_enable_mbx_intr(struct nicpf *nic) { diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c index 5b4d3badcb73..e7ee7005657c 100644 --- a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c +++ b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c @@ -1801,6 +1801,20 @@ void nicvf_clear_intr(struct nicvf *nic, int int_type, int q_idx) nicvf_reg_write(nic, NIC_VF_INT, mask); } +/* Check if interrupt is active */ +int nicvf_check_is_intr_active(struct nicvf *nic, int int_type, int q_idx) +{ + u64 mask = nicvf_int_type_to_mask(int_type, q_idx); + + if (!mask) { + netdev_dbg(nic->netdev, + "Failed to read interrupt status: unknown type\n"); + return 0; + } + + return (mask & nicvf_reg_read(nic, NIC_VF_INT)); +} + /* Check if interrupt is enabled */ int nicvf_is_intr_enabled(struct nicvf *nic, int int_type, int q_idx) { diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_queues.h b/drivers/net/ethernet/cavium/thunder/nicvf_queues.h index 5e9a03cf1b4d..58f6fbe48bce 100644 --- a/drivers/net/ethernet/cavium/thunder/nicvf_queues.h +++ b/drivers/net/ethernet/cavium/thunder/nicvf_queues.h @@ -358,6 +358,7 @@ void nicvf_enable_intr(struct nicvf *nic, int int_type, int q_idx); void nicvf_disable_intr(struct nicvf *nic, int int_type, int q_idx); void nicvf_clear_intr(struct nicvf *nic, int int_type, int q_idx); int nicvf_is_intr_enabled(struct nicvf *nic, int int_type, int q_idx); +int nicvf_check_is_intr_active(struct nicvf *nic, int int_type, int q_idx); /* Register access APIs */ void nicvf_reg_write(struct nicvf *nic, u64 offset, u64 val); -- 2.17.2
[PATCH 3/8] net: thunderx: make CFG_DONE message to run through generic send-ack sequence
At the end of NIC VF initialization it send CFG_DONE message to PF without using nicvf_msg_send_to_pf routine. This potentially could re-write data in mailbox. This commit is to implement common way of sending CFG_DONE message by the same way with other configuration messages by using nicvf_send_msg_to_pf() routine. Signed-off-by: Vadim Lomovtsev --- drivers/net/ethernet/cavium/thunder/nic_main.c | 2 +- .../net/ethernet/cavium/thunder/nicvf_main.c | 18 +++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/cavium/thunder/nic_main.c b/drivers/net/ethernet/cavium/thunder/nic_main.c index 6c8dcb65ff03..90497a27df18 100644 --- a/drivers/net/ethernet/cavium/thunder/nic_main.c +++ b/drivers/net/ethernet/cavium/thunder/nic_main.c @@ -1039,7 +1039,7 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf) case NIC_MBOX_MSG_CFG_DONE: /* Last message of VF config msg sequence */ nic_enable_vf(nic, vf, true); - goto unlock; + break; case NIC_MBOX_MSG_SHUTDOWN: /* First msg in VF teardown sequence */ if (vf >= nic->num_vf_en) diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c index abf24e7dff2d..b0e8a04e0f1e 100644 --- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c +++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c @@ -169,6 +169,20 @@ static int nicvf_check_pf_ready(struct nicvf *nic) return 1; } +static int nicvf_send_cfg_done(struct nicvf *nic) +{ + union nic_mbx mbx = {}; + + mbx.msg.msg = NIC_MBOX_MSG_CFG_DONE; + if (nicvf_send_msg_to_pf(nic, &mbx)) { + netdev_err(nic->netdev, + "PF didn't respond to CFG DONE msg\n"); + return 0; + } + + return 1; +} + static void nicvf_read_bgx_stats(struct nicvf *nic, struct bgx_stats_msg *bgx) { if (bgx->rx) @@ -1416,7 +1430,6 @@ int nicvf_open(struct net_device *netdev) struct nicvf *nic = netdev_priv(netdev); struct queue_set *qs = nic->qs; struct nicvf_cq_poll *cq_poll = NULL; - union nic_mbx mbx = {}; /* wait till all queued set_rx_mode tasks completes if any */ drain_workqueue(nic->nicvf_rx_mode_wq); @@ -1515,8 +1528,7 @@ int nicvf_open(struct net_device *netdev) nicvf_enable_intr(nic, NICVF_INTR_RBDR, qidx); /* Send VF config done msg to PF */ - mbx.msg.msg = NIC_MBOX_MSG_CFG_DONE; - nicvf_write_to_mbx(nic, &mbx); + nicvf_send_cfg_done(nic); return 0; cleanup: -- 2.17.2
[PATCH 5/8] net: thunderx: rework xcast message structure to make it fit into 64 bit
To communicate to PF each of ThunderX NIC VF uses mailbox which is pair of 64 bit registers available to both VFn and PF. This commit is to change the xcast message structure in order to fit it into 64 bit. Signed-off-by: Vadim Lomovtsev --- drivers/net/ethernet/cavium/thunder/nic.h| 6 ++ drivers/net/ethernet/cavium/thunder/nic_main.c | 4 ++-- drivers/net/ethernet/cavium/thunder/nicvf_main.c | 6 +++--- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/drivers/net/ethernet/cavium/thunder/nic.h b/drivers/net/ethernet/cavium/thunder/nic.h index 376a96bce33f..227343625e83 100644 --- a/drivers/net/ethernet/cavium/thunder/nic.h +++ b/drivers/net/ethernet/cavium/thunder/nic.h @@ -577,10 +577,8 @@ struct set_ptp { struct xcast { u8msg; - union { - u8mode; - u64 mac; - } data; + u8mode; + u64 mac:48; }; /* 128 bit shared memory between PF and each VF */ diff --git a/drivers/net/ethernet/cavium/thunder/nic_main.c b/drivers/net/ethernet/cavium/thunder/nic_main.c index 90497a27df18..620dbe082ca0 100644 --- a/drivers/net/ethernet/cavium/thunder/nic_main.c +++ b/drivers/net/ethernet/cavium/thunder/nic_main.c @@ -1094,7 +1094,7 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf) bgx = NIC_GET_BGX_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]); lmac = NIC_GET_LMAC_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]); bgx_set_dmac_cam_filter(nic->node, bgx, lmac, - mbx.xcast.data.mac, + mbx.xcast.mac, vf < NIC_VF_PER_MBX_REG ? vf : vf - NIC_VF_PER_MBX_REG); break; @@ -1106,7 +1106,7 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf) } bgx = NIC_GET_BGX_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]); lmac = NIC_GET_LMAC_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]); - bgx_set_xcast_mode(nic->node, bgx, lmac, mbx.xcast.data.mode); + bgx_set_xcast_mode(nic->node, bgx, lmac, mbx.xcast.mode); break; default: dev_err(&nic->pdev->dev, diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c index dbd8862d60d6..30c7f54b4f17 100644 --- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c +++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c @@ -1964,7 +1964,7 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs, * its' own LMAC to the filter to accept packets for it. */ mbx.xcast.msg = NIC_MBOX_MSG_ADD_MCAST; - mbx.xcast.data.mac = 0; + mbx.xcast.mac = 0; if (nicvf_send_msg_to_pf(nic, &mbx) < 0) goto free_mc; } @@ -1974,7 +1974,7 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs, /* now go through kernel list of MACs and add them one by one */ for (idx = 0; idx < mc_addrs->count; idx++) { mbx.xcast.msg = NIC_MBOX_MSG_ADD_MCAST; - mbx.xcast.data.mac = mc_addrs->mc[idx]; + mbx.xcast.mac = mc_addrs->mc[idx]; if (nicvf_send_msg_to_pf(nic, &mbx) < 0) goto free_mc; } @@ -1982,7 +1982,7 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs, /* and finally set rx mode for PF accordingly */ mbx.xcast.msg = NIC_MBOX_MSG_SET_XCAST; - mbx.xcast.data.mode = mode; + mbx.xcast.mode = mode; nicvf_send_msg_to_pf(nic, &mbx); free_mc: -- 2.17.2
[PATCH] fbdev: mxsfb: implement FB_PRE_INIT_FB option
From: Melchior Franz The FB_PRE_INIT_FB option keeps the kernel from reinitializing the display and prevents flickering during the transition from a bootloader splash screen to the kernel logo screen. Make this option available for the mxsfb driver. Signed-off-by: Melchior Franz Signed-off-by: Martin Kepplinger Signed-off-by: Manfred Schlaegl --- drivers/video/fbdev/Kconfig | 2 +- drivers/video/fbdev/mxsfb.c | 12 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig index 58a9590c9db6..0e7ab29c9c70 100644 --- a/drivers/video/fbdev/Kconfig +++ b/drivers/video/fbdev/Kconfig @@ -2183,7 +2183,7 @@ config FB_EP93XX config FB_PRE_INIT_FB bool "Don't reinitialize, use bootloader's GDC/Display configuration" - depends on FB && FB_MB862XX_LIME + depends on FB && (FB_MB862XX_LIME || FB_MXS) ---help--- Select this option if display contents should be inherited as set by the bootloader. diff --git a/drivers/video/fbdev/mxsfb.c b/drivers/video/fbdev/mxsfb.c index 12c8bd1d24d5..a017200a16b3 100644 --- a/drivers/video/fbdev/mxsfb.c +++ b/drivers/video/fbdev/mxsfb.c @@ -181,6 +181,7 @@ struct mxsfb_info { const struct mxsfb_devdata *devdata; u32 sync; struct regulator *reg_lcd; + int pre_init; }; #define mxsfb_is_v3(host) (host->devdata->ipversion == 3) @@ -419,6 +420,12 @@ static int mxsfb_set_par(struct fb_info *fb_info) fb_info->fix.line_length = line_size; + if (host->pre_init) { + mxsfb_enable_controller(fb_info); + host->pre_init = 0; + return 0; + } + /* * It seems, you can't re-program the controller if it is still running. * This may lead into shifted pictures (FIFO issue?). @@ -931,6 +938,10 @@ static int mxsfb_probe(struct platform_device *pdev) if (IS_ERR(host->reg_lcd)) host->reg_lcd = NULL; +#if defined(CONFIG_FB_PRE_INIT_FB) + host->pre_init = 1; +#endif + fb_info->pseudo_palette = devm_kcalloc(&pdev->dev, 16, sizeof(u32), GFP_KERNEL); if (!fb_info->pseudo_palette) { @@ -963,6 +974,7 @@ static int mxsfb_probe(struct platform_device *pdev) mxsfb_enable_controller(fb_info); } + host->pre_init = 0; dev_info(&pdev->dev, "initialized\n"); return 0; -- 2.20.1 smime.p7s Description: S/MIME cryptographic signature
[PATCH 4/8] net: thunderx: add nicvf_send_msg_to_pf result check for set_rx_mode_task
The rx_set_mode invokes number of messages to be send to PF for receive mode configuration. In case if there any issues we need to stop sending messages and release allocated memory. This commit is to implement check of nicvf_msg_send_to_pf() result. Signed-off-by: Vadim Lomovtsev --- drivers/net/ethernet/cavium/thunder/nicvf_main.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c index b0e8a04e0f1e..dbd8862d60d6 100644 --- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c +++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c @@ -1956,7 +1956,8 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs, /* flush DMAC filters and reset RX mode */ mbx.xcast.msg = NIC_MBOX_MSG_RESET_XCAST; - nicvf_send_msg_to_pf(nic, &mbx); + if (nicvf_send_msg_to_pf(nic, &mbx) < 0) + goto free_mc; if (mode & BGX_XCAST_MCAST_FILTER) { /* once enabling filtering, we need to signal to PF to add @@ -1964,7 +1965,8 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs, */ mbx.xcast.msg = NIC_MBOX_MSG_ADD_MCAST; mbx.xcast.data.mac = 0; - nicvf_send_msg_to_pf(nic, &mbx); + if (nicvf_send_msg_to_pf(nic, &mbx) < 0) + goto free_mc; } /* check if we have any specific MACs to be added to PF DMAC filter */ @@ -1973,9 +1975,9 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs, for (idx = 0; idx < mc_addrs->count; idx++) { mbx.xcast.msg = NIC_MBOX_MSG_ADD_MCAST; mbx.xcast.data.mac = mc_addrs->mc[idx]; - nicvf_send_msg_to_pf(nic, &mbx); + if (nicvf_send_msg_to_pf(nic, &mbx) < 0) + goto free_mc; } - kfree(mc_addrs); } /* and finally set rx mode for PF accordingly */ @@ -1983,6 +1985,8 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs, mbx.xcast.data.mode = mode; nicvf_send_msg_to_pf(nic, &mbx); +free_mc: + kfree(mc_addrs); } static void nicvf_set_rx_mode_task(struct work_struct *work_arg) -- 2.17.2
[PATCH 2/8] net: thunderx: replace global nicvf_rx_mode_wq work queue for all VFs to private for each of them.
Having one work queue for receive mode configuration ndo_set_rx_mode() call for all VFs results in making each of them wait till the set_rx_mode() call completes for another VF if any of close, set receive mode and change flags calls being already invoked. Potentially this could cause device state change before appropriate call of receive mode configuration completes, so the call itself became meaningless, corrupt data or break configuration sequence. We don't need any delays in NIC VF configuration sequence so having delayed work call with 0 delay has no sense. This commit is to implement one work queue for each NIC VF for set_rx_mode task and to let them work independently and replacing delayed_work with work_struct. Signed-off-by: Vadim Lomovtsev --- drivers/net/ethernet/cavium/thunder/nic.h | 4 ++- .../net/ethernet/cavium/thunder/nicvf_main.c | 30 ++- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/drivers/net/ethernet/cavium/thunder/nic.h b/drivers/net/ethernet/cavium/thunder/nic.h index f4d81765221e..376a96bce33f 100644 --- a/drivers/net/ethernet/cavium/thunder/nic.h +++ b/drivers/net/ethernet/cavium/thunder/nic.h @@ -271,7 +271,7 @@ struct xcast_addr_list { }; struct nicvf_work { - struct delayed_workwork; + struct work_struct work; u8 mode; struct xcast_addr_list *mc; }; @@ -327,6 +327,8 @@ struct nicvf { struct nicvf_work rx_mode_work; /* spinlock to protect workqueue arguments from concurrent access */ spinlock_t rx_mode_wq_lock; + /* workqueue for handling kernel ndo_set_rx_mode() calls */ + struct workqueue_struct *nicvf_rx_mode_wq; /* PTP timestamp */ struct cavium_ptp *ptp_clock; diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c index 88f8a8fa93cd..abf24e7dff2d 100644 --- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c +++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c @@ -68,9 +68,6 @@ module_param(cpi_alg, int, 0444); MODULE_PARM_DESC(cpi_alg, "PFC algorithm (0=none, 1=VLAN, 2=VLAN16, 3=IP Diffserv)"); -/* workqueue for handling kernel ndo_set_rx_mode() calls */ -static struct workqueue_struct *nicvf_rx_mode_wq; - static inline u8 nicvf_netdev_qidx(struct nicvf *nic, u8 qidx) { if (nic->sqs_mode) @@ -1311,6 +1308,9 @@ int nicvf_stop(struct net_device *netdev) struct nicvf_cq_poll *cq_poll = NULL; union nic_mbx mbx = {}; + /* wait till all queued set_rx_mode tasks completes */ + drain_workqueue(nic->nicvf_rx_mode_wq); + mbx.msg.msg = NIC_MBOX_MSG_SHUTDOWN; nicvf_send_msg_to_pf(nic, &mbx); @@ -1418,6 +1418,9 @@ int nicvf_open(struct net_device *netdev) struct nicvf_cq_poll *cq_poll = NULL; union nic_mbx mbx = {}; + /* wait till all queued set_rx_mode tasks completes if any */ + drain_workqueue(nic->nicvf_rx_mode_wq); + netif_carrier_off(netdev); err = nicvf_register_misc_interrupt(nic); @@ -1973,7 +1976,7 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs, static void nicvf_set_rx_mode_task(struct work_struct *work_arg) { struct nicvf_work *vf_work = container_of(work_arg, struct nicvf_work, - work.work); + work); struct nicvf *nic = container_of(vf_work, struct nicvf, rx_mode_work); u8 mode; struct xcast_addr_list *mc; @@ -2030,7 +2033,7 @@ static void nicvf_set_rx_mode(struct net_device *netdev) kfree(nic->rx_mode_work.mc); nic->rx_mode_work.mc = mc_list; nic->rx_mode_work.mode = mode; - queue_delayed_work(nicvf_rx_mode_wq, &nic->rx_mode_work.work, 0); + queue_work(nic->nicvf_rx_mode_wq, &nic->rx_mode_work.work); spin_unlock(&nic->rx_mode_wq_lock); } @@ -2187,7 +2190,10 @@ static int nicvf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) INIT_WORK(&nic->reset_task, nicvf_reset_task); - INIT_DELAYED_WORK(&nic->rx_mode_work.work, nicvf_set_rx_mode_task); + nic->nicvf_rx_mode_wq = alloc_ordered_workqueue("nicvf_rx_mode_wq_VF%d", + WQ_MEM_RECLAIM, + nic->vf_id); + INIT_WORK(&nic->rx_mode_work.work, nicvf_set_rx_mode_task); spin_lock_init(&nic->rx_mode_wq_lock); err = register_netdev(netdev); @@ -2228,13 +2234,15 @@ static void nicvf_remove(struct pci_dev *pdev) nic = netdev_priv(netdev); pnetdev = nic->pnicvf->netdev; - cancel_delayed_work_sync(&nic->rx_mode_work.work); - /* Check if this Qset is assigned to different VF. * If yes, clean primary and all secondary Qsets. */ if (pnetdev && (p
Re: [PATCH] usb: dwc3: Enable GBit Ethernet on Odroid XU4
+Marek As Vivek's and Andrzej's Samsung IDs bounced back. On 06/02/19 11:38, Jochen Sprickerhof wrote: > * Roger Quadros [2019-02-06 10:41]: >> Hi, >> >> On 21/01/19 16:02, Jochen Sprickerhof wrote: > [..] >>> I'm not sure why this it only works with the driver compiled into the >>> kernel nor why it needs a hard reset or why it was the line was dropped >>> when the patch was accepted. Would be great to get some feedback of the >>> authors. >> >> When XHCI driver is compiled into the kernel are the relevant PHY drivers >> compiled in as well? > > Only CONFIG_USB_PHY=y (I took the config from the > linux-image-4.19.0-1-armmp_4.19.13-1_armhf.deb Debian package as a basis > where this is the default). > -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Re: [PATCH] ASoC: dapm: Check for NULL widget in dapm_update_dai_unlocked
On 2/6/19 11:05, Charles Keepax wrote: > DAIs linked to the dummy will not have an associated playback/capture > widget, so we need to skip the update in that case. > > Fixes: 078a85f2806f ("ASoC: dapm: Only power up active channels from a DAI") > Signed-off-by: Charles Keepax Tested-by: Sylwester Nawrocki > --- > > Ok so that all makes sense, this patch is probably the best fix? It seems so, everything works well with such change, thank you. > sound/soc/soc-dapm.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c > index 482ddb825fb59..5235d8828758a 100644 > --- a/sound/soc/soc-dapm.c > +++ b/sound/soc/soc-dapm.c > @@ -2570,6 +2570,9 @@ static int dapm_update_dai_unlocked(struct > snd_pcm_substream *substream, > else > w = dai->capture_widget; > > + if (!w) > + return 0; > + > dev_dbg(dai->dev, "Update DAI routes for %s %s\n", dai->name, > dir == SNDRV_PCM_STREAM_PLAYBACK ? "playback" : "capture"); >
Re: [PATCH] ASoC: dapm: Check for NULL widget in dapm_update_dai_unlocked
On Wed, 6 Feb 2019 at 11:05, Charles Keepax wrote: > > DAIs linked to the dummy will not have an associated playback/capture > widget, so we need to skip the update in that case. > > Fixes: 078a85f2806f ("ASoC: dapm: Only power up active channels from a DAI") > Signed-off-by: Charles Keepax > --- > > Ok so that all makes sense, this patch is probably the best fix? > > Thanks, > Charles For this particular issue (NULL-pointer): Reported-by: Krzysztof Kozlowski Tested-by: Krzysztof Kozlowski However now I see bug sleeping in atomic context: [ 64.000828] BUG: sleeping function called from invalid context at ../kernel/locking/mutex.c:908 [ 64.008483] in_atomic(): 1, irqs_disabled(): 128, pid: 353, name: aplay [ 64.014978] 2 locks held by aplay/353: [ 64.018671] #0: 1fb9b63d (&(&group->lock)->rlock){}, at: snd_pcm_action_lock_irq+0x18/0x3c [ 64.027337] #1: 8b42bfe8 (&(&pri_dai->spinlock)->rlock){}, at: i2s_trigger+0x130/0x6c8 [ 64.035654] irq event stamp: 8754 [ 64.038925] hardirqs last enabled at (8753): [] _raw_spin_unlock_irq+0x24/0x5c [ 64.047094] hardirqs last disabled at (8754): [] _raw_spin_lock_irq+0x18/0x50 [ 64.055068] softirqs last enabled at (8096): [] __do_softirq+0x4f0/0x5e4 [ 64.062680] softirqs last disabled at (8083): [] irq_exit+0x160/0x16c [ 64.069953] Preemption disabled at: [ 64.069956] [<>] (null) [ 64.076700] CPU: 6 PID: 353 Comm: aplay Not tainted 5.0.0-rc5-next-20190206-1-g101ffa564f78 #348 [ 64.085822] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) [ 64.091882] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [ 64.099601] [] (show_stack) from [] (dump_stack+0x98/0xc4) [ 64.106788] [] (dump_stack) from [] (___might_sleep+0x264/0x2cc) [ 64.114501] [] (___might_sleep) from [] (__mutex_lock+0x40/0xa98) [ 64.122300] [] (__mutex_lock) from [] (mutex_lock_nested+0x1c/0x24) [ 64.130275] [] (mutex_lock_nested) from [] (clk_prepare_lock+0x50/0xf8) [ 64.138596] [] (clk_prepare_lock) from [] (clk_core_get_rate+0xc/0x60) [ 64.146824] [] (clk_core_get_rate) from [] (i2s_trigger+0x444/0x6c8) [ 64.154883] [] (i2s_trigger) from [] (soc_pcm_trigger+0x100/0x140) [ 64.162768] [] (soc_pcm_trigger) from [] (snd_pcm_action_single+0x38/0x80) [ 64.171349] [] (snd_pcm_action_single) from [] (snd_pcm_action+0x54/0x5c) [ 64.179841] [] (snd_pcm_action) from [] (snd_pcm_action_lock_irq+0x28/0x3c) [ 64.188508] [] (snd_pcm_action_lock_irq) from [] (snd_pcm_ioctl+0x920/0x123c) [ 64.197350] [] (snd_pcm_ioctl) from [] (do_vfs_ioctl+0xb0/0x9f8) [ 64.205054] [] (do_vfs_ioctl) from [] (ksys_ioctl+0x34/0x5c) [ 64.212418] [] (ksys_ioctl) from [] (ret_fast_syscall+0x0/0x28) [ 64.220045] Exception stack(0xe69dffa8 to 0xe69dfff0) [ 64.225058] ffa0: 004391b8 1770 0004 4142 00439340 00439340 [ 64.233218] ffc0: 004391b8 1770 1770 0036 1770 be8db940 [ 64.241361] ffe0: b6fa382c be8db8ec b6f32a3c b6e42bdc [ 64.246386] [ 64.247823] == [ 64.253979] WARNING: possible circular locking dependency detected [ 64.260133] 5.0.0-rc5-next-20190206-1-g101ffa564f78 #348 Tainted: GW [ 64.268193] -- [ 64.274342] aplay/353 is trying to acquire lock: [ 64.278937] 51044846 (prepare_lock){+.+.}, at: clk_prepare_lock+0x50/0xf8 [ 64.285694] [ 64.285694] but task is already holding lock: [ 64.291500] 8b42bfe8 (&(&pri_dai->spinlock)->rlock){}, at: i2s_trigger+0x130/0x6c8 [ 64.299387] [ 64.299387] which lock already depends on the new lock. [ 64.299387] [ 64.307534] [ 64.307534] the existing dependency chain (in reverse order) is: [ 64.314985] [ 64.314985] -> #1 (&(&pri_dai->spinlock)->rlock){}: [ 64.321667]clk_mux_set_parent+0x34/0xb8 [ 64.326162]clk_core_set_parent_nolock+0x21c/0x54c [ 64.331535]clk_set_parent+0x38/0x6c [ 64.335696]of_clk_set_defaults+0x11c/0x384 [ 64.340460]of_clk_add_provider+0x8c/0xc8 [ 64.345054]samsung_i2s_probe+0x484/0x64c [ 64.349651]platform_drv_probe+0x6c/0xa4 [ 64.354153]really_probe+0x280/0x414 [ 64.358311]driver_probe_device+0x78/0x1c4 [ 64.362991]bus_for_each_drv+0x74/0xb8 [ 64.367323]__device_attach+0xd4/0x16c [ 64.371655]bus_probe_device+0x88/0x90 [ 64.375988]deferred_probe_work_func+0x4c/0xd0 [ 64.381017]process_one_work+0x228/0x810 [ 64.385520]worker_thread+0x30/0x570 [ 64.389681]kthread+0x134/0x164 [ 64.393405]ret_from_fork+0x14/0x20 [ 64.397477] (null) [ 64.400249] [ 64.400249] -> #0 (prepare_lock){+.+.}: [ 64.405539]__mutex_lock+0x7c/0xa98 [ 64.409610]mutex_lock_nested+0x1c/0x
Re: [linux-sunxi] Re: [PATCH 1/9] pinctrl: sunxi: Support I/O bias voltage setting on A80
On Wed, Feb 6, 2019 at 4:14 PM Linus Walleij wrote: > > On Wed, Feb 6, 2019 at 4:32 AM Chen-Yu Tsai wrote: > > > The A80 SoC has configuration registers for I/O bias voltage. Incorrect > > settings would make the affected peripherals inoperable in some cases, > > such as Ethernet RGMII signals biased at 2.5V with the settings still > > at 3.3V. However low speed signals such as MDIO on the same group of > > pins seem to be unaffected. > > > > Previously there was no way to know what the actual voltage used was, > > short of hard-coding a value in the device tree. With the new pin bank > > regulator supply support in place, the driver can now query the > > regulator for its voltage, and if it's valid (as opposed to being the > > dummy regulator), set the bias voltage setting accordingly. > > > > Add a quirk to denote the presence of the configuration registers, and > > a function to set the correct setting based on the voltage read back > > from the regulator. > > > > This is only done when the regulator is first acquired and enabled. > > While it would be nice to have a notifier on the regulator so that when > > the voltage changes, the driver can update the setting, in practice no > > board currently supports dynamic changing of the I/O voltages. > > > > Signed-off-by: Chen-Yu Tsai > > Hi Chen-Yu, > > thanks for the patch! I tried to apply it on the pinctrl devel branch > (for v5.1) but it failed to apply, I assume because it depends on > the fixes that I just sent to Torvalds. > > Shall we proceed like this that I merge v5.0-rc6 as soon as it is > out and then try to apply this on top of that instead, so we get > rid of this conflict? Sure. That sounds like a good plan. ChenYu
Re: [PATCH v2 1/7] pinctrl: at91: add option to use drive strength bits
On Wed, Feb 06, 2019 at 09:46:28AM +, claudiu.bez...@microchip.com wrote: > > > On 06.02.2019 10:13, Ludovic Desroches wrote: > > On Thu, Jan 31, 2019 at 05:18:04PM +0100, Claudiu Beznea - M18063 wrote: > >> From: Claudiu Beznea > >> > >> SAM9X60 uses high and low drive strengths. To implement this, in > >> at91_pinctrl_mux_ops::set_drivestrength and > >> at91_pinctrl_mux_ops::get_drivestrength we need bit numbers of > >> drive strengths (1 for low, 2 for high), thus change the code to > >> allow the usage of drive strength bit numbers. > >> > >> Signed-off-by: Claudiu Beznea > >> --- > >> drivers/pinctrl/pinctrl-at91.c | 32 > >> 1 file changed, 20 insertions(+), 12 deletions(-) > >> > >> diff --git a/drivers/pinctrl/pinctrl-at91.c > >> b/drivers/pinctrl/pinctrl-at91.c > >> index 3d49bbbcdbc7..31f06dafca2e 100644 > >> --- a/drivers/pinctrl/pinctrl-at91.c > >> +++ b/drivers/pinctrl/pinctrl-at91.c > >> @@ -72,10 +72,15 @@ static int gpio_banks; > >> * DRIVE_STRENGTH_DEFAULT is just a placeholder to avoid changing the > >> drive > >> * strength when there is no dt config for it. > >> */ > >> -#define DRIVE_STRENGTH_DEFAULT(0 << DRIVE_STRENGTH_SHIFT) > >> -#define DRIVE_STRENGTH_LOW (1 << DRIVE_STRENGTH_SHIFT) > >> -#define DRIVE_STRENGTH_MED (2 << DRIVE_STRENGTH_SHIFT) > >> -#define DRIVE_STRENGTH_HI (3 << DRIVE_STRENGTH_SHIFT) > >> +enum drive_strength_bit { > >> + DRIVE_STRENGTH_BIT_DEF, > >> + DRIVE_STRENGTH_BIT_LOW, > >> + DRIVE_STRENGTH_BIT_MED, > >> + DRIVE_STRENGTH_BIT_HI, > >> +}; > >> + > >> +#define DRIVE_STRENGTH_BIT_MSK(name) (DRIVE_STRENGTH_BIT_##name << \ > >> + DRIVE_STRENGTH_SHIFT) > >> > >> /** > >> * struct at91_pmx_func - describes AT91 pinmux functions > >> @@ -551,7 +556,7 @@ static unsigned > >> at91_mux_sama5d3_get_drivestrength(void __iomem *pio, > >>/* SAMA5 strength is 1:1 with our defines, > >> * except 0 is equivalent to low per datasheet */ > >>if (!tmp) > >> - tmp = DRIVE_STRENGTH_LOW; > >> + tmp = DRIVE_STRENGTH_BIT_MSK(LOW); > >> > >>return tmp; > >> } > >> @@ -564,7 +569,7 @@ static unsigned at91_mux_sam9x5_get_drivestrength(void > >> __iomem *pio, > >> > >>/* strength is inverse in SAM9x5s hardware with the pinctrl defines > >> * hardware: 0 = hi, 1 = med, 2 = low, 3 = rsvd */ > >> - tmp = DRIVE_STRENGTH_HI - tmp; > >> + tmp = DRIVE_STRENGTH_BIT_MSK(HI) - tmp; > >> > >>return tmp; > >> } > >> @@ -600,7 +605,7 @@ static void at91_mux_sam9x5_set_drivestrength(void > >> __iomem *pio, unsigned pin, > >> > >>/* strength is inverse on SAM9x5s with our defines > >> * 0 = hi, 1 = med, 2 = low, 3 = rsvd */ > >> - setting = DRIVE_STRENGTH_HI - setting; > >> + setting = DRIVE_STRENGTH_BIT_MSK(HI) - setting; > >> > >>set_drive_strength(pio + at91sam9x5_get_drive_register(pin), pin, > >>setting); > >> @@ -959,11 +964,11 @@ static int at91_pinconf_set(struct pinctrl_dev > >> *pctldev, > >>} \ > >> } while (0) > >> > >> -#define DBG_SHOW_FLAG_MASKED(mask,flag) do { \ > >> +#define DBG_SHOW_FLAG_MASKED(mask,flag,name) do { \ > > > > checkpatch error: space required > > I'm aware of that but I added it in the way it was previously (were the > checkpatch error was still present). Please let me know if you want me to > send a new version fixing this. I think it's the opportunity to fix it. If you can resend a new version including my ack. Regards Ludovic > > Thank you, > Claudiu Beznea > > > > >>if ((config & mask) == flag) { \ > >>if (num_conf) \ > >>seq_puts(s, "|"); \ > >> - seq_puts(s, #flag); \ > >> + seq_puts(s, #name); \ > >>num_conf++; \ > >>} \ > >> } while (0) > >> @@ -981,9 +986,12 @@ static void at91_pinconf_dbg_show(struct pinctrl_dev > >> *pctldev, > >>DBG_SHOW_FLAG(PULL_DOWN); > >>DBG_SHOW_FLAG(DIS_SCHMIT); > >>DBG_SHOW_FLAG(DEGLITCH); > >> - DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_LOW); > >> - DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_MED); > >> - DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_HI); > >> + DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_BIT_MSK(LOW), > >> + DRIVE_STRENGTH_LOW); > >> + DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_BIT_MSK(MED), > >> + DRIVE_STRENGTH_MED); > >> + DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_BIT_MSK(HI), > >> + DRIVE_STRENGTH_HI); > >>DBG_SHOW_FLAG(DEBOUNCE); > >>if (config & DEBOUNCE) { > >>val = config >> DEBOUNCE_VAL_SHIFT; > >> -- > >> 2.7.4 > >> > > > ___
Re: [PATCH] LSM: Allow syzbot to ignore security= parameter.
On 2019/02/04 17:07, Dmitry Vyukov wrote: > On Fri, Feb 1, 2019 at 2:09 PM Tetsuo Handa > wrote: >> >> On 2019/02/01 19:50, Dmitry Vyukov wrote: >>> On Fri, Feb 1, 2019 at 11:44 AM Tetsuo Handa >>> wrote: On 2019/02/01 19:09, Dmitry Vyukov wrote: > Thanks for the explanations. > > Here is the change that I've come up with: > https://github.com/google/syzkaller/commit/aa53be276dc84aa8b3825b3416542447ff82b41a You are not going to apply this updated config to upstream kernels now, are you? Removing CONFIG_DEFAULT_SECURITY="apparmor" from configs used by upstream kernels will cause failing to enable AppArmor (unless security=apparmor is specified). >>> >>> >>> We do use security=apparmor, see: >>> https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-apparmor.cmdline >>> https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-selinux.cmdline >>> https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-smack.cmdline >>> >> >> Oh, security= parameter is explicitly specified on all targets? >> Then, we can abuse CONFIG_DEBUG_AID_FOR_SYZBOT option. ;-) >> >> LSM folks, may we use this patch for linux-next.git ? >> CONFIG_DEBUG_AID_FOR_SYZBOT is a linux-next.git-only kernel config option >> used by syzbot. > > > Then we also need this on syzbot side, right? Otherwise it seems that > all instances will default to a single security module. > https://github.com/google/syzkaller/commit/ffec3d1894ffd05966b50efa49ca19af76c9ea81 > Right. But as I update the documentation ( https://tomoyo.osdn.jp/2.6/chapter-3.html.en#3.6 ), I came to think that we should ignore security= parameter when lsm= parameter is specified. Currently, it is possible to enable TOMOYO and only one of SELinux/Smack/AppArmor. Therefore, it is possible to disable only TOMOYO by specifying security=selinux when we want to enable only SELinux, by specifying security=smack when we want to enable only Smack, by specifying security=apparmor when we want to enable only AppArmor. That is, we can use security= parameter in order to specify the other LSM module which should not be disabled. But when it becomes possible to enable TOMOYO and more than one of SELinux/Smack/AppArmor, we will no longer be able to selectively disable one LSM module using security= parameter, for security= parameter is intended for specifying only one LSM module which should be enabled. That is, we will need to use lsm= parameter in order to selectively disable LSM modules. Then, I think that it is straightforward (and easier to manage) to ignore security= parameter when lsm= parameter is specified. Furthermore, we could even avoid introducing lsm= parameter by allowing security= parameter to specify multiple LSM modules. For example, security= parameter is interpreted as a list of all LSM modules which should be enabled when it contains a comma, and it is interpreted as one of LSM_FLAG_LEGACY_MAJOR modules which should be enabled otherwise. Then, specifying security=selinux or security=smack or security=tomoyo or security=apparmor or security=none will respectively enable SELinux, Smack, TOMOYO, AppArmor, none of SELinux/Smack/TOMOYO/AppArmor. And specifying e.g. security=, will disable all LSM modules.
Re: [PATCH v2 0/9] gpio: mockup: improve the user-space testing interface
wt., 29 sty 2019 o 09:44 Bartosz Golaszewski napisał(a): > > From: Bartosz Golaszewski > > This series aims at reworking the gpio-mockup debugfs interface. The > reason for that is the fact that certain known problems with this > testing module exist and the user-space tests are broken anyway > after commit fa38869b0161 ("gpiolib: Don't support irq sharing > for userspace") which made it impossible for gpio-mockup to ignore > certain events (e.g. only receive notifications about rising edge > events). > > The first three patches improve the interrupt simulator. The first one > makes the struct irq_chip part of the irq_sim structure so that we can > support multiple instances at once. The second delegates the irq number > mapping to the irq domain subsystem. The third patch provides a helper > that will allow users to specify the type of the fired interrupt so that > the configuration set by the set_type() callback can be taken into account. > > Next six patches improve the gpio-mockup module. Patches 4-5 have been > reviewed before but missed the last merge window. Patch 6 is there > because we're already breaking the debugfs interface anyway and it > removes a link that has no users. Patches 7-8 are minor tweaks. > > Last patch introduces a rework of the debugfs interface. With this > change each mockup chip is represented by a directory named after the > chip's device name under /gpio-mockup/. Each line > is represented by a file using the line's offset as the name under the > chip's directory. Reading from the line's file yields the current > *value*, writing to the line's file changes the current "pull". Default > pull for mockup lines is down. More info on that can be found in the > comment added by this change to the gpio-mockup code. > > This is somewhat inspired by the idea of the gpio-simulator by > Uwe Kleine-König except that I strongly belive that when testing > certain user API code paths we should not be using the same paths for > verification. That's why there's a separate interface (debugfs) sharing > as little as possible with the character device that allows to check if > various operations (reading and setting values, events) work as > expected instead of two connected dummy chips sharing the same > interface. > > If accepted this will of course require major modification of user-space > tests in libgpiod once upstream. > > v1 -> v2: > - instead of providing the irq_sim_get_type() helper, move the irq type > logic into the simulator and provide a helper that allows users to specify > the type of the fired interrupt > > Bartosz Golaszewski (9): > irq/irq_sim: don't share the irq_chip structure between simulators > irq/irq_sim: use irq domain > irq/irq_sim: provide irq_sim_fire_type() > gpio: mockup: add locking > gpio: mockup: implement get_multiple() > gpio: mockup: don't create the debugfs link named after the label > gpio: mockup: change the type of 'offset' to unsigned int > gpio: mockup: change the signature of unlocked get/set helpers > gpio: mockup: rework debugfs interface > > drivers/gpio/gpio-mockup.c | 185 +++-- > include/linux/irq_sim.h| 17 +++- > kernel/irq/irq_sim.c | 126 ++--- > 3 files changed, 263 insertions(+), 65 deletions(-) > > -- > 2.19.1 > Marc, Thomas, if there are no objections - could you please ack the three first patches and let me take them through the GPIO tree? I would really appreciate if we could have them in v5.1. Bart
[PATCH 1/2] PCI: fix serious bug when sizing bridges with additional size
Background: I discovered a bug while preparing my next patch for Thunderbolt native PCI enumeration. This bugfix is vital for that next Thunderbolt patch. They are related and could be put together but I am presenting them as separate patches. Nature of problem: On boot, the PCI bridges are handled in the function pci_assign_unassigned_root_bus_resources(). This function makes multiple passes / attempts to assign all of the resources. It calls __pci_bus_size_bridges() which tries to assign required resources, plus additional requested space. It makes calls to pbus_size_io() and pbus_size_mem() which return zero on success. However, these functions misleadingly return non-zero if the resource is already assigned. The fallout: If the first attempt to allocate resources on a bridge succeeds in 64-bit MMIO_PREF, but fails in 32-bit MMIO, then a second attempt is made. Then, the __pbus_size_mem() returns non-zero for MMIO_PREF because the resource is already assigned. That makes __pci_bus_size_bridges() think there is no space (ENOSPC) and it attempts to assign the sum of the MMIO_PREF and MMIO sizes into a 32-bit window. This means that it tries to request the sum of the the MMIO and MMIO_PREF size into the MMIO window. In the best case, the MMIO size is equal to the MMIO_PREF size and we get double the window in MMIO, and the correct size in MMIO_PREF, and nothing bad happens. In a worse case, doubling the size makes it fail due to lack of space in the miniscule MMIO 32-bit address space. In that case, we might be able to reduce the requested size. In the worst case, we have requested for a vast amount of MMIO_PREF, and a small amount of MMIO. The kernel tries to assign the sum of the sizes of the two types in the MMIO window, leaving us with MMIO_PREF assigned and MMIO unassigned due to the requested size being too large to assign. In this case, there is nothing we can do about it. If a massive additional MMIO_PREF and a small non-zero amount of additional MMIO are needed, then we cannot reduce the requested MMIO_PREF to work around the bug. The cause: pbus_size_io() and pbus_size_mem() both call find_free_bus_resource() to identify which bridge window of the bus meets the criteria. This function explicitly skips resources with a parent, inevitably returning NULL. The functions pbus_size_io() and pbus_size_mem() return -ENOSPC if find_free_bus_resource() returns NULL, meaning an assigned resource is interpreted as having been failed to assign, potentially causing it to try to allocate it again in another window. The solution: my proposed patch renames find_free_bus_resource() to find_bus_resource_of_type() and modifies it to not skip resources with r->parent. The name change is because returning an assigned resource makes the resource potentially not "free". The calling functions, pbus_size_io() and pbus_size_mem() have checks added for b_res->parent and they return 0 (success) in this case. This is because a resource with a parent is already assigned and should be interpreted as a success, not a failure. Testing: I have tested my proposed patch and it appears to work flawlessly. It has the added benefit of slashing the number of attempts taken to assign a given BAR, meaning a much cleaner dmesg. In fact, in my configuration, it has gone from very messy to mainly IO BAR failures. There is nothing that can be done about IO BARs because of the 16-bit hardware limitation, meaning the amount of available space cannot be increased. All that I am left with is a few "failed to assign [mem size]" very early in boot, before those BARs succeed in the next attempt. After the small initial set of failures for MEM, there are no more "failed to assign" whatsoever for non-IO BARs. Remarks: I fixed some other block comments to comply with kernel code styling standards. I had to modify the one block comment to reflect the changes, and checkpatch.pl got really angry and yelled at me. So I figured I may as well fix the lot. Signed-off-by: Nicholas Johnson --- drivers/pci/setup-bus.c | 82 + 1 file changed, 51 insertions(+), 31 deletions(-) diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index a8be1a90f..c7e0a5e2b 100644 --- a/drivers/pci/setup-bus.c +++ b/drivers/pci/setup-bus.c @@ -563,17 +563,19 @@ void pci_setup_cardbus(struct pci_bus *bus) } EXPORT_SYMBOL(pci_setup_cardbus); -/* Initialize bridges with base/limit values we have collected. - PCI-to-PCI Bridge Architecture Specification rev. 1.1 (1998) - requires that if there is no I/O ports or memory behind the - bridge, corresponding range must be turned off by writing base - value greater than limit to the bridge's base/limit registers. - - Note: care must be taken when updating I/O base/limit registers - of bridges which support 32-bit I/O. This update requires two - config space writes, so it's quite possible that an I/O window of - the bridge will have some undesirable address (e.g. 0
[PATCH] net: sxgbe: fix unintended sign extension
From: Colin Ian King Shifting a u8 by 24 will cause the value to be promoted to an integer. If the top bit of the u8 is set then the following conversion to an unsigned long will sign extend the value causing the upper 32 bits to be set in the result. Fix this by casting the u8 value to an unsigned long before the shift. Detected by CoverityScan, CID#1195586 ("Unintended sign extension") Fixes: 1edb9ca69e8a ("net: sxgbe: add basic framework for Samsung 10Gb ethernet driver") Signed-off-by: Colin Ian King --- drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c b/drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c index 6d22dd500790..51fe161e5da9 100644 --- a/drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c +++ b/drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c @@ -1822,7 +1822,8 @@ static void sxgbe_set_umac_addr(void __iomem *ioaddr, unsigned char *addr, * is RO. */ writel(data | SXGBE_HI_REG_AE, ioaddr + SXGBE_ADDR_HIGH(reg_n)); - data = (addr[3] << 24) | (addr[2] << 16) | (addr[1] << 8) | addr[0]; + data = ((unsigned long)addr[3] << 24) | (addr[2] << 16) | + (addr[1] << 8) | addr[0]; writel(data, ioaddr + SXGBE_ADDR_LOW(reg_n)); } -- 2.20.1
[PATCH 2/2] PCI: modify kernel parameters to differentiate between MMIO and MMIO_PREF sizes
Depends: commit a1140e7bcb10ff96c192ee200e6cbf832f27158e ("PCI: fix serious bug when sizing bridges with additional size") Background: I have come to find that the kernel parameters for reserving resources for the hotplug bridges are not useful for Thunderbolt with native PCI enumeration. You can only increase the size so far before it fails to allocate the 32-bit MMIO windows. Nature of problem: pci=hpmemsize=nnM is parsed from drivers/pci/pci.c and used in drivers/pci/setup-bus.c. It overwrites pci_hotplug_mem_size which has a default value set by DEFAULT_HOTPLUG_MEM_SIZE. When used, this value is used to request additional space for MMIO and MMIO_PREF in equal amounts. The fallout: if you increase the value of pci=hpmemsize=nnM to be too large to fit into the 32-bit address space, the MMIO window fails to allocate and has zero-sized BARs. In the case of Thunderbolt, this means the NHI does not have the resources needed to function, and the thunderbolt.ko driver fails to probe the device. All Thunderbolt functionality is lost. Even if the NHI worked, any Thunderbolt device containing endpoints with 32-bit BARs (most of them) would fail to initialise properly. The worst case happens if some resources end up allocating with non-zero size, but too small. With some AMD Radeon external GPUs, if certain combinations of BARs are assigned / failed to assign, it can cause BUG_ON from arch/x86/mm/pat.c due to the buggy amdgpu.ko driver trying to use a zero-sized BAR, instead of failing gracefully. This more or less renders the system unusable. The cause: because the kernel parameters do not allow the user to specify the MMIO and MMIO_PREF hotplug bridge additional sizes separately, they have no choice but to give unrealistic sizes that will fail, or will be forced to make unpleasant compromises, resulting in having to tolerate unstable or unpredictable operation. The solution: My proposed patch depreciates pci=hpmemsize=nnM,hpiosize=nn but allows them to work without any change in user-visible functionality, with a KERN_WARNING message when either are used. It adds the new parameters pci=hp_io_size=nn,hp_mmio_size=nnM,hp_mmio_pref_size=nnM. If any of the new kernel parameters are used, all of the newly-depreciated ones will be overridden. This will allow for a grace period to allow people to change to the new kernel parameters without unpleasant disruption. At a time deemed appropriate by kernel developers, the old parameters can be easily removed without the need to rework any code. My testing: This works very well. I have not encountered any problems and I am happily allocating 128M/64G under each Thunderbolt port with nocrs. The success is dependent on my previous bug patch, which I decided to separate from this feature patch. The old functionality still works the same, and is overridden by the new commands if they are used. Remarks: The printk at the end of pci_setup() in drivers/pci/pci.c has a strange backquote ('`', under the tilde on the keyboard key) in the format. I am not sure if it is a typo, or deliberate, and whether it should change or if my KERN_WARNING logs need to use that symbol. Also, the scripts/checkpatch.pl berates me for using printk, but the alternative, pci_warn(), requires a pci_dev to work, and this message does not pertain to a particular device. Signed-off-by: Nicholas Johnson --- .../admin-guide/kernel-parameters.txt | 21 -- drivers/pci/pci.c | 39 +-- drivers/pci/setup-bus.c | 26 +++-- include/linux/pci.h | 3 +- 4 files changed, 69 insertions(+), 20 deletions(-) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index b799bcf67..284752ff7 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -3327,12 +3327,27 @@ the default. off: Turn ECRC off on: Turn ECRC on. - hpiosize=nn[KMG]The fixed amount of bus space which is + + hpiosize=nn[KMG]Depreciated. Overridden by hp_io_size + value if any of hp_io_size, hp_mmio_size + or hp_mmio_pref_size are used. This sets + hp_io_size to the given value if not + overridden. + + hpmemsize=nn[KMG] Depreciated. Overridden if any of + hp_io_size, hp_mmio_size or + hp_mmio_pref_size are used. This sets + hp_mmio_size and hp_mmio_pref_size to + the given value if not overridden. + hp_io_size=nn[KMG] The fixed amount of bus space which is reserved for hotplug
Re: [PATCH v10 00/25] arm64: provide pseudo NMI with GICv3
Hi Julien, On Thu, Jan 31, 2019 at 02:58:38PM +, Julien Thierry wrote: > This series is a continuation of the work started by Daniel [1]. The goal > is to use GICv3 interrupt priorities to simulate an NMI. > > The patches depend on the core API for NMIs patches [2]. Both series can > be found on this branch: > git clone http://linux-arm.org/linux-jt.git -b v5.0-pseudo-nmi I queued these patches in the arm64 for-next/core, on top of Marc's generic-nmi branch: git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms irq/generic-nmi I'll push the changes out later today, once my tests finished. Thanks for pushing this series through. -- Catalin
Re: [PATCH] reset: Don't WARN if trying to get a used reset control
On Tue, 2019-02-05 at 23:13 +0100, Thierry Reding wrote: [...] > > Drivers that never call _acquire()/_release() must continue to work as > > they are, so exclusive reset controls have to be acquired by default. > > I don't think they have to. See below. Currently the API makes guarantees about the state a reset line is in after an assert() or deassert() call. I'm thinking of the following scenario: driver A driver B request_exclusive() deassert() | request_exclusive() | acquire() | assert() | driver A expects reset | driver B expects reset line | line to stay deasserted| to stay asserted during this | during this time | this time | release() assert() If driver A deassert() succeeds because the control refcnt is still 1, and driver B acquire() succeeds because driver A didn't explicitly acquire the control, driver B assert() should succeed as well and that would cause a conflict without either driver getting an error. Also, how to decide at driver B request_exclusive() time whether or not to throw a warning? We the core doesn't know at this point whether driver B will use acquire()/release(). [...] > > If the reset line is kept asserted while the power domain is turned off, > > it could indeed stay acquired by the power domain. > > That's at least the way that it works on Tegra. Not sure if that is > universally applicable, though it seems logical to me from a hardware > point of view. It may not be required to keep the reset asserted while > the power domain is turned off, but I'd be surprised if there was a > requirement that it be deasserted while the power domain is off. There is hardware that doesn't allow to control the level of the reset line, providing only a self clearing bit that can be used to trigger a reset pulse (via reset_control_reset()). > > > Hm... actually this means that power domains would have to request the > > > reset controls and call reset_control_acquire() during probe. That way > > > when the domain is powered on it will release the reset and free it up > > > for the SOR driver to use. > > > > If you can guarantee that the power domain is powered up and releases > > the reset control before the SOR driver tries to request the reset line, > > that would work. > > Well, that's the part that the acquire/release protocol would enforce. Only if both drivers use it. I'd prefer the API to provide consistent behaviour even if one of the drivers doesn't use acquire/release. > If your device shares a reset control with a power domain, this should > work fine, provided that the device driver uses the reset only between > (or during) ->runtime_resume() and ->runtime_suspend(). There should be a clear error path in case drivers don't use the reset control only during suspend/resume as expected. > > > This seems right because it also happens to work from a runtime PM > > > sequencing point of view. > > > > > > I think the problem for single-user exclusive reset controls could be > > > solved by making use of the reference counting that's already used by > > > shared resets. We should be able to go ahead and use the reference > > > counting in general, and the acquire()/release() dance would only have > > > to be enforced when the reference count is > 1. > > > > I'm not sure I understand what you mean by making use of the shared > > resets' refcounting. I suppose you could check the reset_control's > > refcnt to determine whether there are other users. Or did you want to > > repurpose deassert_count? > > Yes, if we make the new acquire/release protocol only apply if the > reset control's refcnt is > 1, then I think we should be able to keep > exclusive resets working exactly the same way they are now. Since the > exclusive resets are guaranteed to have refcnt == 1 right now, there > should be no need for them to be implicitly acquired at request time. Ok, thanks for the explanation. That is correct, but it would break the guarantee we give for exclusive reset controls, that the state of the reset line stays exactly as requested. refcnt could change at any time due to a second driver being probed, that could acquire and (de)assert the reset line. > However, when you start using them from multiple users, the acquire/ > release protocol would kick in and refuse to assert/deassert before > you've acquired them. > > I think we should also be able to keep the WARN_ON(), albeit maybe in > different form. We could have one if the user tries to acquire a reset > control that's already acquired. And we could also have a warning if we > assert or deassert a reset control that hasn't been acquired *and* that > has refcnt > 1. With the above I think "acquired" really needs to mean > acquired && refcnt > 1, and then the current exclusive resets should > work out of the box. How would you resolve the above scenario? Implicitly acquiring exc
Re: [PATCH v2] livepatch: core: Return EOPNOTSUPP instead of ENOSYS
On Tue 2019-02-05 09:59:33, Josh Poimboeuf wrote: > On Tue, Feb 05, 2019 at 03:33:28AM +0900, Alice Ferrazzi wrote: > > From: Alice Ferrazzi > > > > As a result of an unsupported operation is better to use EOPNOTSUPP > > as error code. > > ENOSYS is only used for 'invalid syscall nr' and nothing else. > > > > Signed-off-by: Alice Ferrazzi > > Acked-by: Josh Poimboeuf I have applied the patch into for-5.1/atomic-replace branch. Best Regards, Petr
Re: [PATCH 0/2] PM-runtime: fix and clean up of time accounting
On Monday, February 4, 2019 5:25:51 PM CET Vincent Guittot wrote: > Fix time accounting which has the same lock contraint as for using hrtimer > and update accounting_timestamp only when useful. > > Vincent Guittot (2): > PM-runtime: move runtime accounting on ktime_get_mono_fast_ns() > PM-runtime: update time accounting only when enabled > > drivers/base/power/runtime.c | 25 ++--- > 1 file changed, 18 insertions(+), 7 deletions(-) > > Both applied, thanks!
Re: [PATCH v2 0/4] livepatch: Followup changes for the atomic replace patchset
On Mon 2019-02-04 14:56:49, Petr Mladek wrote: > This patchset implements ideas that were mentioned and postponed during > the review of the atomic replace patchset. > > The patches apply on top of livepatching.git, branch > origin/for-5.1/atomic-replace. > > > Changes against v1: > > + Added Joe's patch that fixed ptr_id() error code [Joe] > + Did proper error handling in the shadow variable sefttest [All] > + Removed the controversial patch that was removing patch->enabled flag > [All]. > + Fixed few typo's [Joe] > + Added available Acks. > > > Joe Lawrence (1): > livepatch: return -ENOMEM on ptr_id() allocation failure > > Petr Mladek (3): > livepatch: Introduce klp_for_each_patch macro > livepatch: Proper error handling in the shadow variables selftest > livepatch: Module coming and going callbacks can proceed with all > listed patches I have applied all 4 paches into for-5.1/atomic-replace branch. Best Regards, Petr
Re: [RFC PATCH 7/8] KVM: i8254: Remove need for irq ack notifier
On 04/02/19 15:42, Suthikulpanit, Suravee wrote: > From: Julian Stecklina > > ACK notifiers don't work with AMD AVIC when the PIT interrupt is > delivered as edge-triggered fixed interrupt via the IOAPIC. AMD > processors cannot exit on EOI for these interrupts. The ACK notifiers do > work when the interrupt is delivered via PIC as ExtINT, because the ACK > comes as PIO write that KVM sees. > > Change the PIT code to not rely on the ACK notifiers. The IRQ ACK > notifier in the PIT emulation re-schedules pit->expired to reinject any > pending PIT interrupt. This seems useless, because we can pulse the PIT > interrupt even when the interrupt is not ACKed yet. This means any timer > expiry when the interrupt was being handled by the guest, will cause an > interrupt to be injected automatically when the interrupt is ACKed. The difference is that with the irq ack notifier *all* the pending PIT interrupts will be delivered. Without, only one will. If you don't want the ack notifier, you can do that with the KVM_REINJECT_CONTROL ioctl. Unfortunately, reinject is enabled by default and that is part of the userspace ABI and we cannot change it. It's possible to enable AVIC when reinject is disabled, and we fix that at the QEMU level by changing the default reinject behavior, basically with this one-line change: diff --git a/hw/i386/kvm/i8254.c b/hw/i386/kvm/i8254.c index d4d4a85..1d82860 100644 --- a/hw/i386/kvm/i8254.c +++ b/hw/i386/kvm/i8254.c @@ -305,7 +305,7 @@ static void kvm_pit_realizefn(DeviceState *dev, Error **errp) static Property kvm_pit_properties[] = { DEFINE_PROP_UINT32("iobase", PITCommonState, iobase, -1), DEFINE_PROP_LOSTTICKPOLICY("lost_tick_policy", KVMPITState, - lost_tick_policy, LOST_TICK_POLICY_DELAY), + lost_tick_policy, LOST_TICK_POLICY_DISCARD), DEFINE_PROP_END_OF_LIST(), }; plus the corresponding change to compat properties in hw/i386/pc.c. Alternatively, it is probably a good time to switch the default to split irqchip in QEMU. Split irqchip was introduced in kernel 4.5, which was released about three years ago. Paolo > Reviewed-by: Filippo Sironi > Signed-off-by: Julian Stecklina > Signed-off-by: Suravee Suthikulpanit
Re: [PATCH 1/2] cpufreq: stats: Declare freq-attr right after their callbacks
On Friday, February 1, 2019 7:15:44 AM CET Viresh Kumar wrote: > Freq attribute for "trans_table" is defined right after its callback > (without any blank line between them), but the others are defined > separately later on. Keep this consistent and define all attributes > right after their callbacks. > > Signed-off-by: Viresh Kumar > --- > drivers/cpufreq/cpufreq_stats.c | 7 +++ > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c > index 1572129844a5..941e63e3e652 100644 > --- a/drivers/cpufreq/cpufreq_stats.c > +++ b/drivers/cpufreq/cpufreq_stats.c > @@ -51,6 +51,7 @@ static ssize_t show_total_trans(struct cpufreq_policy > *policy, char *buf) > { > return sprintf(buf, "%d\n", policy->stats->total_trans); > } > +cpufreq_freq_attr_ro(total_trans); > > static ssize_t show_time_in_state(struct cpufreq_policy *policy, char *buf) > { > @@ -69,6 +70,7 @@ static ssize_t show_time_in_state(struct cpufreq_policy > *policy, char *buf) > } > return len; > } > +cpufreq_freq_attr_ro(time_in_state); > > static ssize_t store_reset(struct cpufreq_policy *policy, const char *buf, > size_t count) > @@ -77,6 +79,7 @@ static ssize_t store_reset(struct cpufreq_policy *policy, > const char *buf, > cpufreq_stats_clear_table(policy->stats); > return count; > } > +cpufreq_freq_attr_wo(reset); > > static ssize_t show_trans_table(struct cpufreq_policy *policy, char *buf) > { > @@ -126,10 +129,6 @@ static ssize_t show_trans_table(struct cpufreq_policy > *policy, char *buf) > } > cpufreq_freq_attr_ro(trans_table); > > -cpufreq_freq_attr_ro(total_trans); > -cpufreq_freq_attr_ro(time_in_state); > -cpufreq_freq_attr_wo(reset); > - > static struct attribute *default_attrs[] = { > &total_trans.attr, > &time_in_state.attr, > Applied, along with the [2/2], thanks!
[PATCH] rtc: pm8xxx: fix unintended sign extension
From: Colin Ian King Shifting a u8 by 24 will cause the value to be promoted to an integer. If the top bit of the u8 is set then the following conversion to an unsigned long will sign extend the value causing the upper 32 bits to be set in the result. Fix this by casting the u8 value to an unsigned long before the shift. Detected by CoverityScan, CID#1309693 ("Unintended sign extension") Fixes: 9a9a54ad7aa2 ("drivers/rtc: add support for Qualcomm PMIC8xxx RTC") Signed-off-by: Colin Ian King --- drivers/rtc/rtc-pm8xxx.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c index 1074e3dbfc1d..cda020700744 100644 --- a/drivers/rtc/rtc-pm8xxx.c +++ b/drivers/rtc/rtc-pm8xxx.c @@ -213,7 +213,8 @@ static int pm8xxx_rtc_read_time(struct device *dev, struct rtc_time *tm) } } - secs = value[0] | (value[1] << 8) | (value[2] << 16) | (value[3] << 24); + secs = value[0] | (value[1] << 8) | (value[2] << 16) | + ((unsigned long)value[3] << 24); rtc_time_to_tm(secs, tm); @@ -284,7 +285,8 @@ static int pm8xxx_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm) return rc; } - secs = value[0] | (value[1] << 8) | (value[2] << 16) | (value[3] << 24); + secs = value[0] | (value[1] << 8) | (value[2] << 16) | + ((unsigned long)value[3] << 24); rtc_time_to_tm(secs, &alarm->time); -- 2.20.1
Re: [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed
On Monday, February 4, 2019 4:37:20 PM CET Sudeep Holla wrote: > On Thu, Jan 31, 2019 at 04:05:59PM +, Sudeep Holla wrote: > > On Thu, Jan 31, 2019 at 12:48:49AM +0100, Rafael J. Wysocki wrote: > > > On Friday, January 25, 2019 4:09:06 PM CET Sudeep Holla wrote: > > > > The sysfs for the cpu caches are managed by adding devices with cpu > > > > as the parent in cpu_device_create() when secondary cpu is brought > > > > onlin. Generally when the secondary CPUs are hotplugged back is as part > > > > of resume from suspend-to-ram, we call cpu_device_create() from the cpu > > > > hotplug state machine while the cpu device associated with that CPU is > > > > not yet ready to be resumed as the device_resume() call happens bit > > > > later. > > > > It's not really needed to set the flag is_prepared for cpu devices are > > > > they are mostly pseudo device and hotplug framework deals with state > > > > machine and not managed through the cpu device. > > > > > > > > This often results in annoying warning when resuming: > > > > Enabling non-boot CPUs ... > > > > CPU1: Booted secondary processor > > > > cache: parent cpu1 should not be sleeping > > > > CPU1 is up > > > > CPU2: Booted secondary processor > > > > cache: parent cpu2 should not be sleeping > > > > CPU2 is up > > > > and so on. > > > > > > > > Just fix the warning by updating the device state quite early. > > > > > > > > Cc: "Rafael J. Wysocki" > > > > Reported-by: Jisheng Zhang > > > > Reported-by: Steve Longerbeam > > > > Reported-by: Eugeniu Rosca > > > > Signed-off-by: Sudeep Holla > > > > --- > > > > drivers/base/cpu.c | 20 +++- > > > > include/linux/cpuhotplug.h | 1 + > > > > 2 files changed, 20 insertions(+), 1 deletion(-) > > > > > > > > Hi Rafael, > > > > > > > > This is getting reported for quite some time. Let me know if you have > > > > better solution to fix this harmless yet annoying warnings during system > > > > resume. > > > > > > I'd rather have a flag in struct dev_pm_info that will cause the message > > > to > > > be suppressed if set. > > > > > > It could be used for other purposes too then in princple (like skipping > > > the > > > creation of empty "power" attr groups in sysfs for devices that don't > > > need them etc.). > > > > > Thanks for the suggestion. I did quick hack and came up with something > > below. I wanted to run through you once before I materialise it into > > a formal patch to check if I understood your suggestion correctly. > > We can move no_pm_required outside dev_pm_info struct and rename with > > any better names. > > > > Sorry for the nag, since the title has RFC, thought there are chances of > this getting lost. Let me know if the below idea aligns with your suggestion ? RFC would be fine, but Patchwork doesn't pick up patches posted as replies in the middle of a thread. :-) Yes, this is basically what I suggested, please post. Cheers, Rafael
Re: [PATCH v5 0/9] cpufreq: Add flag to auto-register as cooling device
On Tuesday, January 29, 2019 5:55:06 AM CET Amit Kucheria wrote: > Add a flag for cpufreq drivers to tell cpufreq core to auto-register > themselves as a thermal cooling device. > > There series converts over all the drivers except arm_big_little.c. > Tested on SDM845 with the qcom-cpufreq-hw driver. Only compile-tested the > others. > > Things needing fixing (but not a blocker for the series): > - Look at how to detect that we're not in IKS mode in arm_big_little's >.ready callback. > > Changes since v4: > - Added IS_ENABLED guards in cpufreq.c > - Changed flag name to CPUFREQ_IS_COOLING_DEV > - Collected various review tags > > Changes since v3: > - Got rid of wrapper function to register/unregister cooling devices. >Directly call the function in cpufreq.c > > Changes since v2: > - Get rid of #ifdef'ery and let the pointer exist in all cases > - Get rid of (!CPU_THERMAL || THERMAL) dependency in all cpufreq drivers' >Kconfig > > Changes since v1: > - Fix compilation failures with allmodconfig > - Get rid of #ifdef in cpufreq.c > - Removed miscellaneous patches and sent them separately > - Merged patches 1 and 2 from v1 > > Amit Kucheria (9): > thermal: cpu_cooling: Require thermal core to be compiled in > cpufreq: Auto-register the driver as a thermal cooling device if asked > cpufreq: qcom-hw: Register as a cpufreq cooling device > cpufreq: imx6q: Use auto-registration of thermal cooling device > cpufreq: cpufreq-dt: Use auto-registration of thermal cooling device > cpufreq: mediatek: Use auto-registration of thermal cooling device > cpufreq: qoriq: Use auto-registration of thermal cooling device > cpufreq: scmi: Use auto-registration of thermal cooling device > cpufreq: scpi: Use auto-registration of thermal cooling device > > drivers/cpufreq/Kconfig| 3 --- > drivers/cpufreq/Kconfig.arm| 5 - > drivers/cpufreq/cpufreq-dt.c | 14 ++ > drivers/cpufreq/cpufreq.c | 13 + > drivers/cpufreq/imx6q-cpufreq.c| 24 ++-- > drivers/cpufreq/mediatek-cpufreq.c | 14 ++ > drivers/cpufreq/qcom-cpufreq-hw.c | 3 ++- > drivers/cpufreq/qoriq-cpufreq.c| 15 ++- > drivers/cpufreq/scmi-cpufreq.c | 14 ++ > drivers/cpufreq/scpi-cpufreq.c | 14 ++ > drivers/thermal/Kconfig| 1 + > include/linux/cpufreq.h| 9 + > 12 files changed, 37 insertions(+), 92 deletions(-) > > I've applied the series, thanks!
[PATCH v4 1/1] arm64: dts: sdm845: wireup the thermal trip points to cpufreq
Since all cpus in the big and little clusters, respectively, are in the same frequency domain, use all of them for mitigation in the cooling-map. We end up with two cooling devices - one each for the big and little clusters. Signed-off-by: Amit Kucheria Acked-by: Eduardo Valentin --- arch/arm64/boot/dts/qcom/sdm845.dtsi | 241 --- 1 file changed, 217 insertions(+), 24 deletions(-) diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index c27cbd3bcb0a..8ce5bf55a345 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -13,6 +13,7 @@ #include #include #include +#include / { interrupt-parent = <&intc>; @@ -99,6 +100,7 @@ compatible = "qcom,kryo385"; reg = <0x0 0x0>; enable-method = "psci"; + #cooling-cells = <2>; next-level-cache = <&L2_0>; L2_0: l2-cache { compatible = "cache"; @@ -114,6 +116,7 @@ compatible = "qcom,kryo385"; reg = <0x0 0x100>; enable-method = "psci"; + #cooling-cells = <2>; next-level-cache = <&L2_100>; L2_100: l2-cache { compatible = "cache"; @@ -126,6 +129,7 @@ compatible = "qcom,kryo385"; reg = <0x0 0x200>; enable-method = "psci"; + #cooling-cells = <2>; next-level-cache = <&L2_200>; L2_200: l2-cache { compatible = "cache"; @@ -138,6 +142,7 @@ compatible = "qcom,kryo385"; reg = <0x0 0x300>; enable-method = "psci"; + #cooling-cells = <2>; next-level-cache = <&L2_300>; L2_300: l2-cache { compatible = "cache"; @@ -150,6 +155,7 @@ compatible = "qcom,kryo385"; reg = <0x0 0x400>; enable-method = "psci"; + #cooling-cells = <2>; next-level-cache = <&L2_400>; L2_400: l2-cache { compatible = "cache"; @@ -162,6 +168,7 @@ compatible = "qcom,kryo385"; reg = <0x0 0x500>; enable-method = "psci"; + #cooling-cells = <2>; next-level-cache = <&L2_500>; L2_500: l2-cache { compatible = "cache"; @@ -174,6 +181,7 @@ compatible = "qcom,kryo385"; reg = <0x0 0x600>; enable-method = "psci"; + #cooling-cells = <2>; next-level-cache = <&L2_600>; L2_600: l2-cache { compatible = "cache"; @@ -186,6 +194,7 @@ compatible = "qcom,kryo385"; reg = <0x0 0x700>; enable-method = "psci"; + #cooling-cells = <2>; next-level-cache = <&L2_700>; L2_700: l2-cache { compatible = "cache"; @@ -1691,18 +1700,41 @@ thermal-sensors = <&tsens0 1>; trips { - cpu_alert0: trip0 { - temperature = <75000>; + cpu0_alert0: trip-point@0 { + temperature = <9>; hysteresis = <2000>; type = "passive"; }; - cpu_crit0: trip1 { + cpu0_alert1: trip-point@1 { + temperature = <95000>; + hysteresis = <2000>; + type = "passive"; + }; + + cpu0_crit: cpu_crit { temperature = <11>; hysteresis = <1000>; type = "critical"; }; }; + + cooling-maps { + map0 { + trip = <&cpu0_alert0>; + cooling-device = <&CPU0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>, +
[PATCH v4 0/1] Thermal throttling for SDM845
Only one patch left in this series. This patch depends on [1] and [2] which have been merged through by Rafael through the power tree. Changes since v3: - first trip point increased to 90 degrees based on Matthias' testing - Added Acks Changes since v2: - Split up the series into auto-register series for cpufreq[1] and miscellaneous patches[2][3]. [1] and [2] are needed for this patch to work. - Merged the two DT patches into one. We end up with 2 passive and 1 critical trip point. - Remove unncessary cooling map for critical trip point, we're shutting down at that trip, duh! [1] https://lore.kernel.org/lkml/cover.1548084260.git.amit.kuche...@linaro.org/ [2] https://lore.kernel.org/patchwork/patch/1033953/ [3] https://lore.kernel.org/patchwork/cover/1033970/ Amit Kucheria (1): arm64: dts: sdm845: wireup the thermal trip points to cpufreq arch/arm64/boot/dts/qcom/sdm845.dtsi | 241 --- 1 file changed, 217 insertions(+), 24 deletions(-) -- 2.17.1
Re: [PATCH] PM/runtime: Optimize pm_runtime_autosuspend_expiration()
On Wednesday, January 30, 2019 10:40:17 PM CET Ladislav Michl wrote: > pm_runtime_autosuspend_expiration calls ktime_get_mono_fast_ns > even when its returned value may be unused. Therefore get > current time later and remove gotos while there. > > Signed-off-by: Ladislav Michl > Acked-by: Tony Lindgren > Acked-by: Vincent Guittot > --- > This patch is based on top of bleeding-edge branch, where > "[PATCH v2 ] PM-runtime: fix deadlock with ktime" is sitting. > I expect v3 of that patch, which should not harm this one. > It is meant to replace "PM/runtime: Do not needlessly call ktime_get" > sent earlier. > > drivers/base/power/runtime.c | 19 --- > 1 file changed, 8 insertions(+), 11 deletions(-) > > diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c > index 65e2b5f48e0c..7bbe28faca8d 100644 > --- a/drivers/base/power/runtime.c > +++ b/drivers/base/power/runtime.c > @@ -145,24 +145,21 @@ static void pm_runtime_cancel_pending(struct device > *dev) > u64 pm_runtime_autosuspend_expiration(struct device *dev) > { > int autosuspend_delay; > - u64 last_busy, expires = 0; > - u64 now = ktime_get_mono_fast_ns(); > + u64 expires; > > if (!dev->power.use_autosuspend) > - goto out; > + return 0; > > autosuspend_delay = READ_ONCE(dev->power.autosuspend_delay); > if (autosuspend_delay < 0) > - goto out; > - > - last_busy = READ_ONCE(dev->power.last_busy); > + return 0; > > - expires = last_busy + (u64)autosuspend_delay * NSEC_PER_MSEC; > - if (expires <= now) > - expires = 0;/* Already expired. */ > + expires = READ_ONCE(dev->power.last_busy); > + expires += (u64)autosuspend_delay * NSEC_PER_MSEC; > + if (expires > ktime_get_mono_fast_ns()) > + return expires; /* Expires in the future */ > > - out: > - return expires; > + return 0; > } > EXPORT_SYMBOL_GPL(pm_runtime_autosuspend_expiration); > > Applied, thanks!
Re: [PATCH v3 1/1] arm64: dts: sdm845: wireup the thermal trip points to cpufreq
On Sat, Jan 26, 2019 at 3:50 AM Matthias Kaehlcke wrote: > > > > trips { > > > > - cpu_alert0: trip0 { > > > > + cpu0_alert1: trip-point@0 { > > > > temperature = <75000>; > > > > > > In my observations a 'switch on/threshold' temperature of 75 degrees > > > leads to aggressive throttling with IPA when the temperature is above > > > this threshold: > > > > > > [ 716.760804] cpu_cooling_ratelimit: 31 callbacks suppressed > > > [ 716.760836] cpu cpu4: Cooling state set to 10. New max freq = 192 > > > [ 716.773390] power_allocator_ratelimit: 15 callbacks suppressed > > > [ 716.773405] thermal thermal_zone5: Controlling power: > > > control_temp=95000 last_temp=73500, curr_temp=75200 > > > total_requested_power=39025 total_granted_power=18654 > > > [ 749.609336] cpu_cooling_ratelimit: 45 callbacks suppressed > > > [ 749.609371] cpu cpu4: Cooling state set to 11. New max freq = 1843200 > > > [ 749.624300] power_allocator_ratelimit: 24 callbacks suppressed > > > [ 749.624323] thermal thermal_zone5: Controlling power: > > > control_temp=95000 last_temp=70800, curr_temp=77200 > > > total_requested_power=40136 total_granted_power=17402 > > > [ 780.152633] cpu_cooling_ratelimit: 41 callbacks suppressed > > > [ 780.152666] cpu cpu4: Cooling state set to 11. New max freq = 1843200 > > > [ 780.165247] power_allocator_ratelimit: 21 callbacks suppressed > > > [ 780.165261] thermal thermal_zone5: Controlling power: > > > control_temp=95000 last_temp=64800, curr_temp=76900 > > > total_requested_power=39719 total_granted_power=1759 > > > > > > (the logs come from a local patch in our tree: > > > https://chromium.googlesource.com/chromiumos/third_party/kernel/+/ec1c501a8093fed44a6697a5913ef2765f518e1f) > > > > > > At this point I don't have a clear idea what would be a reasonable > > > value for the 'switch on/threshold' temperature, but probably it > > > should to be higher than 75 degrees, at least with IPA. If there is > > > no reasonable common configuration for different thermal governors I > > > guess we'll have to target a commonly used governor and systems > > > using other 'incompatible' governors need to override the config in > > > their .dtsi. > > Thanks for the elaborate testing and for sharing the numbers. This is very useful information. > > On my system I don't see a significant delta in core temperatures for > > 'threshold' temperatures of 80, 85 or 90°C. However Dhrystone > > performance goes up by ~8% when changing the trip point from 80 to > > 85°C. For a switch from 85 to 90°C I see a ~2% performance delta. For > > all trip points the average core temperatures are ~80°C (silver) and > > ~85°C (gold). Interestingly I observed the highest average > > temperatures with the trip point at 80°C (repeated measurements were > > taken for different temperatures). > > > > Supposedly LMH throttling is disabled in the firmware I used for > > these tests, however data suggests that it is still active > > (temperature doesn't rise beyond 95°C, even without throttling in > > Linux; Dhrystone performance drops when raising the temperature beyond > > 95°C with a heat gun. I will do some more testing when I get my hands > > on a FW that effectively disables LMH (or raises the threshold to > > something like 105°C). > > > > From the data collected so far I'd suggest a 'threshold' temperature > > of 90°C or if that seems to high 85°C. Behavior might be different > > with other thermal governors or without LMH throttling.. > > Some more data from measurements with different trips points, for the > IPA and the Fair Share governors, LMH throttling was enabled: > > IPA > Dhrystone Temp Silver Temp Gold > 75 6M 78.484.9 > 80 6.21M 81.489.8 > 85 6.74M 81.788.2 > 90 6.88M 79.484.6 > > Fair Share > Dhrystone Temp Silver Temp Gold > 75 6.63M 80.188.5 > 80 6.71M 80.188.5 > 85 6.77M 81.187.8 > 90 7.12M 81.287.8 Interesting that you get more MIPs out of fair share governor when compared to IPA across the board. What devices were providing energy cost information (dynamic-power-coefficient) to the IPA engine? Just CPU and GPU? Can you point me to those patches in gerrit? > Within this range the 'threshold' temperature doesn't seem to have a > large impact on the average CPU temperature. There is a bit of > fluctuation between individual measurements, I wouldn't be surprised > if the outliers of Temp Gold for 75 and 90°C converged more with the > other values with some more measurements. > > I learned how to effectively disable LMH throttling, however with that > it was fairly easy to have the CPUs overheat, even wi
Re: [PATCH v15 0/8] ASoC: sun4i-codec: Add Line-In, FM-In, Mic 2
On Wed, Jan 30, 2019 at 10:26 PM Danny Milosavljevic wrote: > > This patchset adds some mixer controls to sun4i-codec for the Allwinner A10 > and the Allwinner A20. > > It also adds the PGA for the MIC2 preamp. > > Where possible, it uses SOC_DAPM_DOUBLE in order to cut down on the number > of distinct controls in alsamixer. > > v15 changes compared to v14 are: > - Instead of adding controls dynamically, add an extra sun7i_codec_controls >structure and duplicate the sun4i_codec_controls there, with small changes. > - Split Mic Playback Volume into extra patch. > - LEFT OFF adding Capture Selects since it's unclear how we want to structure >it. > - LEFT OFF adding Differential Line Source since it might also change if the >Capture Select implementation is different. Thanks for staying on this. This looks all good now. > v14 changes compared to v13 are: > - Merged some of the patches together if it made sense to test them together. > - Use snd_soc_component_driver. > - Moved SUN4I_CODEC_DAC_ACTL_LFMS, SUN4I_CODEC_DAC_ACTL_RFMS to the correct >patch in the series. > - Kept "Left Mixer Left DAC Playback Switch", "Right Mixer Left DAC >Playback Switch" and "Right Mixer Right DAC Playback Switch" unchanged >compared to the released version - for backward compatibility. > > v13 changes compared to v12 are: > - Added my "Signed-off-by". > - Clarified some commit message text. > > v12 changes compared to v11 are: > - Split up patchset in another way. > - Renamed "Mic1 Capture Volume" to "Mic1 Boost Volume". > - Renamed "Mic2 Capture Volume" to "Mic2 Boost Volume". > - Renamed "Line Capture Volume" to "Line Boost Volume". > - Renamed "Differential Line Capture Switch" to "Differential Line Source". > > v11 changes compared to v10 are: > - Split up patchset. > - Fixed typo in Differential Line Capture Switch. > - Renamed "Non-Differential" value to "Stereo". > - Removed duplicate PA Volume mixer control. > > v10 changes compared to v9 are: > - Use SOC_DAPM_DOUBLE where possible and it makes sense in order to cut >down on the number of controls. > > v9 changes compared to v8 are: > - added Line Differential Capture Switch. > - split Capture Source into Left Capture Select, Right Capture Select. > - added Line Capture Volume. > - rename "sun4i_codec_widgets" to "sun4i_codec_controls" for >consistency with the struct field it's used in. > - rename "Line-In" to "Line". > - rename "Power Amplifier Playback Volume" to "Headphone Playback Volume". > > v8 changes compared to v7 are: > - fixed the routes for line and mic capturing. > > v7 changes compared to v6 are: > - preparation for different A20, A10 controls is now in an extra patch. > - all register definitions are now at the top. > - sun7i-specific things (A20-specific things) are now less >grouped-together. > - rename "Power Amplifier Volume" to "Power Amplifier Playback Volume". > > v6 changes compared to v5 are: > - Mic preamplifier special cases for A20 and A10 now are now not >icky: There are two different _widget arrays and the probe() function >now selects the right one to pass to snd_soc_register_codec() unmodified. > - sun7i-specific things (A20-specific things) are now grouped together. > > v5 changes compared to v4 are: > - Mic preamplifier controls have more common names now. > - Mic preamplifier scale has a 0 dB entry as well now, as documented in the >A20 user manual. > - Mic preamplifier has special cases for A20 and A10 now. > - Gain controls have "Gain" in the name now. > > v4 changes compared to v3 are: > - names of the input are not uppercase anymore. > - bit index constants are now named as in the A20 user manual v1.4. > - added Mic1-In, Mac2-In. > - added Mic1 and Mic2 Pre-Amplifiers. > > v3 changes compared to v2 are: > - added DAPM routes. > > v2 changes compared to v1 are: > - moved Line-In and FM-In playback switches to their respective >sun4i_codec_*_mixer_controls. > > v1 changes: > - added linein, fmin output volumes and switches. > > Danny Milosavljevic (8): > ASoC: sun4i-codec: Add MIC2 Pre-Amplifier, Mic2 > ASoC: sun4i-codec: Add Mic Playback Volume > ASoC: sun4i-codec: Add sun7i_codec_controls, sun7i_codec_codec. > ASoC: sun4i-codec: Add Mic1 Boost Volume, Mic2 Boost Volume > ASoC: sun4i-codec: Merge sun4i_codec_left_mixer_controls and > sun4i_codec_right_mixer_controls into sun4i_codec_mixer_controls > ASoC: sun4i-codec: Add Mic1 Playback Switch, Mic2 Playback Switch > ASoC: sun4i-codec: Add FM Playback Volume, FM Left, FM Right, FM > Playback Switch > ASoC: sun4i-codec: Add Line Playback Volume, Line Boost Volume, Line > Right, Line Left, Line Playback Switch The whole series is Reviewed-by: Chen-Yu Tsai
[PATCH 0/5] perf intel-pt: A few fixes
Hi Here are some fixes and a minor improvement for Intel PT. The first 3 patches are cc stable, but the last 2 are not. Adrian Hunter (5): perf auxtrace: Define auxtrace record alignment perf intel-pt: Fix overlap calculation for padding perf intel-pt: Fix CYC timestamp calculation after OVF perf intel-pt: Packet splitting can happen only on 32-bit perf auxtrace: Add timestamp to auxtrace errors tools/perf/util/auxtrace.c | 26 --- tools/perf/util/auxtrace.h | 5 ++- tools/perf/util/event.h| 3 +- tools/perf/util/intel-bts.c| 4 +-- .../perf/util/intel-pt-decoder/intel-pt-decoder.c | 39 +++--- tools/perf/util/intel-pt.c | 23 + tools/perf/util/s390-cpumsf.c | 7 ++-- tools/perf/util/session.c | 3 ++ 8 files changed, 88 insertions(+), 22 deletions(-) Regards Adrian
[PATCH 4/5] perf intel-pt: Packet splitting can happen only on 32-bit
Data is copied when the trace is stopped, so packets are never split between buffers except when processing if the buffer cannot fit in the address space which can only happen on 32-bit systems. Change the logic to reflect that. Signed-off-by: Adrian Hunter --- tools/perf/util/intel-pt-decoder/intel-pt-decoder.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c index a54d6c9a4601..6e03db142091 100644 --- a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c +++ b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c @@ -868,7 +868,7 @@ static int intel_pt_get_next_packet(struct intel_pt_decoder *decoder) ret = intel_pt_get_packet(decoder->buf, decoder->len, &decoder->packet); - if (ret == INTEL_PT_NEED_MORE_BYTES && + if (ret == INTEL_PT_NEED_MORE_BYTES && BITS_PER_LONG == 32 && decoder->len < INTEL_PT_PKT_MAX_SZ && !decoder->next_buf) { ret = intel_pt_get_split_packet(decoder); if (ret < 0) -- 2.17.1
[PATCH 5/5] perf auxtrace: Add timestamp to auxtrace errors
The timestamp can use useful to find part of a trace that has an error without outputting all of the trace e.g. using the itrace 's' option to skip initial number of events. Signed-off-by: Adrian Hunter --- tools/perf/util/auxtrace.c| 22 -- tools/perf/util/auxtrace.h| 2 +- tools/perf/util/event.h | 3 ++- tools/perf/util/intel-bts.c | 4 ++-- tools/perf/util/intel-pt.c| 23 --- tools/perf/util/s390-cpumsf.c | 7 --- tools/perf/util/session.c | 3 +++ 7 files changed, 48 insertions(+), 16 deletions(-) diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c index ad186d3255d1..267e54df511b 100644 --- a/tools/perf/util/auxtrace.c +++ b/tools/perf/util/auxtrace.c @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -858,7 +859,7 @@ void auxtrace_buffer__free(struct auxtrace_buffer *buffer) void auxtrace_synth_error(struct auxtrace_error_event *auxtrace_error, int type, int code, int cpu, pid_t pid, pid_t tid, u64 ip, - const char *msg) + const char *msg, u64 timestamp) { size_t size; @@ -870,7 +871,9 @@ void auxtrace_synth_error(struct auxtrace_error_event *auxtrace_error, int type, auxtrace_error->cpu = cpu; auxtrace_error->pid = pid; auxtrace_error->tid = tid; + auxtrace_error->fmt = 1; auxtrace_error->ip = ip; + auxtrace_error->time = timestamp; strlcpy(auxtrace_error->msg, msg, MAX_AUXTRACE_ERROR_MSG); size = (void *)auxtrace_error->msg - (void *)auxtrace_error + @@ -1160,12 +1163,27 @@ static const char *auxtrace_error_name(int type) size_t perf_event__fprintf_auxtrace_error(union perf_event *event, FILE *fp) { struct auxtrace_error_event *e = &event->auxtrace_error; + unsigned long long nsecs = e->time; + const char *msg = e->msg; int ret; ret = fprintf(fp, " %s error type %u", auxtrace_error_name(e->type), e->type); + + if (e->fmt && nsecs) { + unsigned long secs = nsecs / NSEC_PER_SEC; + + nsecs -= secs * NSEC_PER_SEC; + ret += fprintf(fp, " time %lu.%09llu", secs, nsecs); + } else { + ret += fprintf(fp, " time 0"); + } + + if (!e->fmt) + msg = (const char *)&e->time; + ret += fprintf(fp, " cpu %d pid %d tid %d ip %#"PRIx64" code %u: %s\n", - e->cpu, e->pid, e->tid, e->ip, e->code, e->msg); + e->cpu, e->pid, e->tid, e->ip, e->code, msg); return ret; } diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h index fac32482db61..c69bcd9a3091 100644 --- a/tools/perf/util/auxtrace.h +++ b/tools/perf/util/auxtrace.h @@ -519,7 +519,7 @@ void auxtrace_index__free(struct list_head *head); void auxtrace_synth_error(struct auxtrace_error_event *auxtrace_error, int type, int code, int cpu, pid_t pid, pid_t tid, u64 ip, - const char *msg); + const char *msg, u64 timestamp); int perf_event__synthesize_auxtrace_info(struct auxtrace_record *itr, struct perf_tool *tool, diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h index feba1aa819b4..36ae7e92dab1 100644 --- a/tools/perf/util/event.h +++ b/tools/perf/util/event.h @@ -532,8 +532,9 @@ struct auxtrace_error_event { u32 cpu; u32 pid; u32 tid; - u32 reserved__; /* For alignment */ + u32 fmt; u64 ip; + u64 time; char msg[MAX_AUXTRACE_ERROR_MSG]; }; diff --git a/tools/perf/util/intel-bts.c b/tools/perf/util/intel-bts.c index f99ac0cbe3ff..0c0180c67574 100644 --- a/tools/perf/util/intel-bts.c +++ b/tools/perf/util/intel-bts.c @@ -144,7 +144,7 @@ static int intel_bts_lost(struct intel_bts *bts, struct perf_sample *sample) auxtrace_synth_error(&event.auxtrace_error, PERF_AUXTRACE_ERROR_ITRACE, INTEL_BTS_ERR_LOST, sample->cpu, sample->pid, -sample->tid, 0, "Lost trace data"); +sample->tid, 0, "Lost trace data", sample->time); err = perf_session__deliver_synth_event(bts->session, &event, NULL); if (err) @@ -374,7 +374,7 @@ static int intel_bts_synth_error(struct intel_bts *bts, int cpu, pid_t pid, auxtrace_synth_error(&event.auxtrace_error, PERF_AUXTRACE_ERROR_ITRACE, INTEL_BTS_ERR_NOINSN, cpu, pid, tid, ip, -"Failed to get instruction"); +"Failed to get instruction", 0); err = perf_session__deliver_synth_event(bts->session, &event, NULL); if (err) diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c index 2e72373ec6df..3b497bab4324 100644 --- a
[PATCH 1/5] perf auxtrace: Define auxtrace record alignment
Define auxtrace record alignment so that it can be referenced elsewhere. Note this is preparation for patch "perf intel-pt: Fix overlap calculation for padding" Signed-off-by: Adrian Hunter Cc: sta...@vger.kernel.org --- tools/perf/util/auxtrace.c | 4 ++-- tools/perf/util/auxtrace.h | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c index 94a22cc8004c..ad186d3255d1 100644 --- a/tools/perf/util/auxtrace.c +++ b/tools/perf/util/auxtrace.c @@ -1279,9 +1279,9 @@ static int __auxtrace_mmap__read(struct perf_mmap *map, } /* padding must be written by fn() e.g. record__process_auxtrace() */ - padding = size & 7; + padding = size & (PERF_AUXTRACE_RECORD_ALIGNMENT - 1); if (padding) - padding = 8 - padding; + padding = PERF_AUXTRACE_RECORD_ALIGNMENT - padding; memset(&ev, 0, sizeof(ev)); ev.auxtrace.header.type = PERF_RECORD_AUXTRACE; diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h index 8e50f96d4b23..fac32482db61 100644 --- a/tools/perf/util/auxtrace.h +++ b/tools/perf/util/auxtrace.h @@ -40,6 +40,9 @@ struct record_opts; struct auxtrace_info_event; struct events_stats; +/* Auxtrace records must have the same alignment as perf event records */ +#define PERF_AUXTRACE_RECORD_ALIGNMENT 8 + enum auxtrace_type { PERF_AUXTRACE_UNKNOWN, PERF_AUXTRACE_INTEL_PT, -- 2.17.1