[GIT PULL] Mailbox changes for v5.1
Hi Linus, The following changes since commit 3717f613f48df0222311f974cf8a06c8a6c97bae: Merge branch 'core-rcu-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip (2019-03-05 14:49:11 -0800) are available in the Git repository at: git://git.linaro.org/landing-teams/working/fujitsu/integration.git tags/mailbox-v5.1 for you to fetch changes up to 17b860bbfc844a3d8e38135ef430d4af8e436b9e: mailbox: imx: keep MU irq working during suspend/resume (2019-03-11 02:51:43 -0500) - mailbox-test: support multiple controller instances - misc cleanup: IMX, STM32 and Tegra - new driver: ZynqMP IPI Anson Huang (1): mailbox: imx: keep MU irq working during suspend/resume Arnd Bergmann (1): mailbox: tegra-hsp: mark suspend function as __maybe_unused Fabien Dessenne (4): mailbox: mailbox-test: fix debugfs in multi-instances mailbox: mailbox-test: fix null pointer if no mmio mailbox: stm32-ipcc: do not enable wakeup source by default mailbox: stm32-ipcc: remove useless device_init_wakeup call Wendy Liang (2): mailbox: ZynqMP IPI mailbox controller dt-bindings: mailbox: Add Xilinx IPI Mailbox .../bindings/mailbox/xlnx,zynqmp-ipi-mailbox.txt | 127 drivers/mailbox/Kconfig| 11 + drivers/mailbox/Makefile | 2 + drivers/mailbox/imx-mailbox.c | 4 +- drivers/mailbox/mailbox-test.c | 26 +- drivers/mailbox/stm32-ipcc.c | 4 +- drivers/mailbox/tegra-hsp.c| 2 +- drivers/mailbox/zynqmp-ipi-mailbox.c | 725 + include/linux/mailbox/zynqmp-ipi-message.h | 20 + 9 files changed, 903 insertions(+), 18 deletions(-) create mode 100644 Documentation/devicetree/bindings/mailbox/xlnx,zynqmp-ipi-mailbox.txt create mode 100644 drivers/mailbox/zynqmp-ipi-mailbox.c create mode 100644 include/linux/mailbox/zynqmp-ipi-message.h
Re: [PATCH v2 0/5] auxdisplay: Introduce charlcd_free()
On Tue, Mar 12, 2019 at 3:44 PM Andy Shevchenko wrote: > > I have found a memory leak in hd44780 and it becomes that we have no > counterpart to charlcd_alloc() that developers can easily miss. Side-note now that I see these patches: I forgot to CC you in a series for charlcd that we got from Mans a week or two ago -- please take a chance to review them if you want! Also pinging Geert & Willy again in case they want to take a look. Cheers, Miguel
Re: [PATCH] drivers: gpio: octeon: use devm_platform_ioremap_resource()
On 12.03.19 14:45, Bartosz Golaszewski wrote: > Can you make this a part of the bigger series and resend together with > subject line fixes? tried to resend it as reply on the prev version. but somehow this didn't seem to work as intented. > Also: maybe consider adding a coccinelle script for that. When I added > that function I noticed there are 1200+ instances in the kernel that > need fixing. I think we'll be better off automating it. haven't coped w/ this yet ... I'll first need to have a deeper look at it. advices welcomed. --mtx -- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering i...@metux.net -- +49-151-27565287
Re: [PATCH v3 1/4] dt: lm3532: Add lm3532 dt doc and update ti_lmu doc
Rob On 3/12/19 9:55 AM, Rob Herring wrote: > On Tue, Mar 12, 2019 at 07:18:19AM -0500, Dan Murphy wrote: >> Add the lm3532 device tree documentation. >> Remove lm3532 device tree reference from the ti_lmu devicetree >> documentation. >> >> With the addition of the dedicated lm3532 documentation the device >> can be removed from the ti_lmu.txt. >> >> The reason for this is that the lm3532 dt documentation now defines >> the ability to control LED output strings against different control >> banks or groups multiple strings to be controlled by a single control >> bank. >> >> Another addition was for ALS lighting control and configuration. The >> LM3532 has a feature that can take in the ALS reading from 2 separate >> ALS devices and adjust the brightness on the strings that are configured >> to support this feature. >> >> Finally the device specific properties were moved to the parent node as these >> properties are not control bank configurable. These include the runtime ramp >> and the ALS configuration. >> >> Signed-off-by: Dan Murphy >> --- >> >> v3 - No changes - https://lore.kernel.org/patchwork/patch/1049026/ >> >> v2 - Fixed ramp-up and ramp-down properties, removed hard coded property >> values, >> added ranges for variable properties, I did not change the label - >> https://lore.kernel.org/patchwork/patch/1048805/ >> >> .../devicetree/bindings/leds/leds-lm3532.txt | 127 ++ >> .../devicetree/bindings/mfd/ti-lmu.txt| 20 --- >> 2 files changed, 127 insertions(+), 20 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3532.txt >> >> diff --git a/Documentation/devicetree/bindings/leds/leds-lm3532.txt >> b/Documentation/devicetree/bindings/leds/leds-lm3532.txt >> new file mode 100644 >> index ..b267d696b511 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/leds/leds-lm3532.txt >> @@ -0,0 +1,127 @@ >> +* Texas Instruments - lm3532 White LED driver with ambient light sensing >> +capability. >> + >> +The LM3532 provides the 3 high-voltage, low-side current sinks. The device >> is >> +programmable over an I2C-compatible interface and has independent >> +current control for all three channels. The adaptive current regulation >> +method allows for different LED currents in each current sink thus allowing >> +for a wide variety of backlight and keypad applications. >> + >> +The main features of the LM3532 include dual ambient light sensor inputs >> +each with 32 internal voltage setting resistors, 8-bit logarithmic and >> linear >> +brightness control, dual external PWM brightness control inputs, and up to >> +1000:1 dimming ratio with programmable fade in and fade out settings. >> + >> +Required properties: >> +- compatible : "ti,lm3532" >> +- reg : I2C slave address >> +- #address-cells : 1 >> +- #size-cells : 0 >> + >> +Required child properties: >> +- reg : Indicates control bank the LED string is controlled by >> +- led-sources : see Documentation/devicetree/bindings/leds/common.txt >> +- ti,led-mode : Defines if the LED strings are manually controlled or >> +if the LED strings are controlled by the ALS. >> +0x00 - LED strings are I2C controlled via full scale >> + brightness control register >> +0x01 - LED strings are ALS controlled >> + >> +Optional child properties: >> +Range for ramp settings: 8us - 65536us >> +- ramp-up-us - The Run time ramp rates/step are from one current >> + set-point to another after the device has reached its >> + initial target set point from turn-on >> +- ramp-down-us - The Run time ramp rates/step are from one current >> + set-point to another after the device has reached its >> + initial target set point from turn-on >> + >> +Optional child properties if ALS mode is used: > > I think you want to remove 'child' here. > Yes they need to be in the parent since they are device and not string specific configurations. > Is this all als properties or none or any combination are valid? > They are all optional in combination. If the value is not set then the default is taken >> +- als-vmin - Minimum ALS voltage defined in Volts >> +- als-vmax - Maximum ALS voltage defined in Volts >> +Per the data sheet the max ALS voltage is 2V and the min is 0V >> + >> +- als1-imp-sel - ALS1 impedance resistor selection in Ohms >> +- als2-imp-sel - ALS2 impedance resistor selection in Ohms >> +Range for impedance select: 37000 Ohms - 1190 Ohms >> +Values above 37kohms will be set to the "High Impedance" setting >> + >> +- als-avrg-time-us - Determines the length of time the device needs to >> + average the two ALS inputs. This is only used if >> + the input mode is LM3532_ALS_INPUT_AVRG. >> + Range:
Re: [PATCH v2 5/5] auxdisplay: hd44780: Convert to use charlcd_free()
On Tue, Mar 12, 2019 at 3:46 PM Andy Shevchenko
wrote:
> Convert to use charlcd_free() instead of kfree() for sake of type check.
>
> Signed-off-by: Andy Shevchenko
Reviewed-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 1/2] Staging: rtl8723bs: os_dep: Fix several coding style errors
On Tue, Mar 12, 2019 at 11:39:13AM -0300, Guilherme T Maeoka wrote: > From: Guilherme T Maeoka > > Fix coding style errors and warns complained by checkpatck.pl. To list: > > - remove braces for single statements blocks, > - add space required around operators, > - replace spaces with tabs to indent, > - add blank line after declarations, > - fix braces and 'else' poistions in selection statements. > I'm sorry you're going to have to break this up into multiple patches. Probably one for each item on your list. regards, dan carpenter
Re: [PATCH v2 4/5] auxdisplay: panel: Convert to use charlcd_free()
On Tue, Mar 12, 2019 at 3:46 PM Andy Shevchenko
wrote:
> Convert to use charlcd_free() instead of kfree() for sake of type check.
>
> Signed-off-by: Andy Shevchenko
Reviewed-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v2 3/5] auxdisplay: charlcd: Introduce charlcd_free() helper
On Tue, Mar 12, 2019 at 3:44 PM Andy Shevchenko
wrote:
> The charlcd_free() is a counterpart to charlcd_alloc()
> and should be called symmetrically on tear down.
>
> Cc: Geert Uytterhoeven
> Signed-off-by: Andy Shevchenko
Reviewed-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v2 2/5] auxdisplay: charlcd: Move to_priv() to charlcd namespace
Hi Andy,
On Tue, Mar 12, 2019 at 3:44 PM Andy Shevchenko
wrote:
> In order to be more particular in names, rename to_priv() macro
> to charlcd_to_priv().
As this is a macro, not a function, the name doesn't end up as a symbol in
the binary anyway, and it's for internal use by the driver only.
> No functional change intended.
>
> Cc: Geert Uytterhoeven
> Signed-off-by: Andy Shevchenko
Regardless:
Reviewed-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 9/9] drivers: ata: sata_rcar: use devm_platform_ioremap_resource()
On 03/12/2019 12:19 PM, Enrico Weigelt, metux IT consult wrote: > Use the new helper that wraps the calls to platform_get_resource() > and devm_ioremap_resource() together. > > Signed-off-by: Enrico Weigelt, metux IT consult Reviewed-by: Sergei Shtylyov [...] MBR, Sergei
[PATCH] pinctrl: baytrail: Fix potential NULL pointer dereference
saved-context in byt_gpio_probe is allocated via devm_kcalloc and is
used without checking for NULL in later functions. This patch avoids
such a scenario.
Signed-off-by: Aditya Pakki
---
drivers/pinctrl/intel/pinctrl-baytrail.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/pinctrl/intel/pinctrl-baytrail.c
b/drivers/pinctrl/intel/pinctrl-baytrail.c
index 241384ead4ed..18d9ad504194 100644
--- a/drivers/pinctrl/intel/pinctrl-baytrail.c
+++ b/drivers/pinctrl/intel/pinctrl-baytrail.c
@@ -1710,6 +1710,8 @@ static int byt_gpio_probe(struct byt_gpio *vg)
#ifdef CONFIG_PM_SLEEP
vg->saved_context = devm_kcalloc(>pdev->dev, gc->ngpio,
sizeof(*vg->saved_context), GFP_KERNEL);
+ if (!vg->saved_context)
+ return -ENOMEM;
#endif
ret = devm_gpiochip_add_data(>pdev->dev, gc, vg);
if (ret) {
--
2.17.1
Re: [RESEND PATCH] mm/hotplug: don't reset pagetype flags for offline
On Sun 10-03-19 16:01:02, Qian Cai wrote:
> The commit f1dd2cd13c4b ("mm, memory_hotplug: do not associate hotadded
> memory to zones until online") introduced move_pfn_range_to_zone() which
> calls memmap_init_zone() during onlining a memory block.
> memmap_init_zone() will reset pagetype flags and makes migrate type to
> be MOVABLE.
>
> However, in __offline_pages(), it also call undo_isolate_page_range()
> after offline_isolated_pages() to do the same thing. Due to
> the commit 2ce13640b3f4 ("mm: __first_valid_page skip over offline
> pages") changed __first_valid_page() to skip offline pages,
> undo_isolate_page_range() here just waste CPU cycles looping around the
> offlining PFN range while doing nothing, because __first_valid_page()
> will return NULL as offline_isolated_pages() has already marked all
> memory sections within the pfn range as offline via
> offline_mem_sections().
>
> Also, after calling the "useless" undo_isolate_page_range() here, it
> reaches the point of no returning by notifying MEM_OFFLINE. Those pages
> will be marked as MIGRATE_MOVABLE again once onlining. In addition, fix
> an incorrect comment along the way.
>
> Signed-off-by: Qian Cai
Well spotted. Thanks!
Acked-by: Michal Hocko
> ---
> mm/memory_hotplug.c | 2 --
> mm/sparse.c | 2 +-
> 2 files changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 6b05576fb4ec..46017040b2f8 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1655,8 +1655,6 @@ static int __ref __offline_pages(unsigned long
> start_pfn,
> /* Ok, all of our target is isolated.
> We cannot do rollback at this point. */
> offline_isolated_pages(start_pfn, end_pfn);
> - /* reset pagetype flags and makes migrate type to be MOVABLE */
> - undo_isolate_page_range(start_pfn, end_pfn, MIGRATE_MOVABLE);
> /* removal success */
> adjust_managed_page_count(pfn_to_page(start_pfn), -offlined_pages);
> zone->present_pages -= offlined_pages;
> diff --git a/mm/sparse.c b/mm/sparse.c
> index 77a0554fa5bd..b3771f35a0ed 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -556,7 +556,7 @@ void online_mem_sections(unsigned long start_pfn,
> unsigned long end_pfn)
> }
>
> #ifdef CONFIG_MEMORY_HOTREMOVE
> -/* Mark all memory sections within the pfn range as online */
> +/* Mark all memory sections within the pfn range as offline */
> void offline_mem_sections(unsigned long start_pfn, unsigned long end_pfn)
> {
> unsigned long pfn;
> --
> 2.17.2 (Apple Git-113)
--
Michal Hocko
SUSE Labs
Re: [PATCH v2 1/5] auxdisplay: hd44780: Fix memory leak on ->remove()
On Tue, Mar 12, 2019 at 3:44 PM Andy Shevchenko
wrote:
> We have to free on ->remove() the allocated resources on ->probe().
>
> Fixes: d47d88361fee ("auxdisplay: Add HD44780 Character LCD support")
> Cc: Geert Uytterhoeven
> Signed-off-by: Andy Shevchenko
Reviewed-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH tip/core/rcu 06/19] rcu: Add warning to detect half-interrupts
On Mon, Mar 11, 2019 at 03:29:03PM -0700, Paul E. McKenney wrote:
> On Mon, Mar 11, 2019 at 09:39:39AM -0400, Joel Fernandes wrote:
> > On Wed, Aug 29, 2018 at 03:20:34PM -0700, Paul E. McKenney wrote:
> > > RCU's dyntick-idle code is written to tolerate half-interrupts, that it,
> > > either an interrupt that invokes rcu_irq_enter() but never invokes the
> > > corresponding rcu_irq_exit() on the one hand, or an interrupt that never
> > > invokes rcu_irq_enter() but does invoke the "corresponding" rcu_irq_exit()
> > > on the other. These things really did happen at one time, as evidenced
> > > by this ca-2011 LKML post:
> > >
> > > http://lkml.kernel.org/r/20111014170019.ge2...@linux.vnet.ibm.com
> > >
> > > The reason why RCU tolerates half-interrupts is that usermode helpers
> > > used exceptions to invoke a system call from within the kernel such that
> > > the system call did a normal return (not a return from exception) to
> > > the calling context. This caused rcu_irq_enter() to be invoked without
> > > a matching rcu_irq_exit(). However, usermode helpers have since been
> > > rewritten to make much more housebroken use of workqueues, kernel threads,
> > > and do_execve(), and therefore should no longer produce half-interrupts.
> > > No one knows of any other source of half-interrupts, but then again,
> > > no one seems insane enough to go audit the entire kernel to verify that
> > > half-interrupts really are a relic of the past.
> > >
> > > This commit therefore adds a pair of WARN_ON_ONCE() calls that will
> > > trigger in the presence of half interrupts, which the code will continue
> > > to handle correctly. If neither of these WARN_ON_ONCE() trigger by
> > > mid-2021, then perhaps RCU can stop handling half-interrupts, which
> > > would be a considerable simplification.
> >
> > Hi Paul and everyone,
> > I was thinking some more about this patch and whether we can simplify this
> > code
> > much in 2021. Since 2021 is a bit far away, I thought working on it in
> > again to
> > keep it fresh in memory is a good idea ;-)
>
> Indeed, easy to forget. ;-)
>
> > To me it seems we cannot easily combine the counters (dynticks_nesting and
> > dynticks_nmi_nesting) even if we confirmed that there is no possibility of a
> > half-interrupt scenario (assuming simplication means counter combining like
> > Byungchul tried to do in https://goo.gl/X1U77X). The reason is because these
> > 2 counters need to be tracked separately as they are used differently in the
> > following function:
> >
> > static int rcu_is_cpu_rrupt_from_idle(void)
> > {
> > return __this_cpu_read(rcu_data.dynticks_nesting) <= 0 &&
> >__this_cpu_read(rcu_data.dynticks_nmi_nesting) <= 1;
> > }
> >
> > dynticks_nesting actually tracks if we entered/exited idle or user mode.
>
> True, though it tracks user mode only in CONFIG_NO_HZ_FULL kernels.
>
> > dynticks_nmi_nesting tracks if we entered/exited interrupts.
>
> Including NMIs, yes.
>
> > We have to do the "dynticks_nmi_nesting <= 1" check because
> > rcu_is_cpu_rrupt_from_idle() can possibly be called from an interrupt itself
> > (like timer) so we discount 1 interrupt, and, the "dynticks_nesting <= 0"
> > check is because the CPU MUST be in user or idle for the check to return
> > true. We can't really combine these two into one counter then I think
> > because
> > they both convey different messages.
> >
> > The only simplication we can do, is probably the "crowbar" updates to
> > dynticks_nmi_nesting can be removed from rcu_eqs_enter/exit once we confirm
> > no more half-interrupts are possible. Which might still be a worthwhile
> > thing
> > to do (while still keeping both counters separate).
> >
> > However, I think we could combine the counters and lead to simplying the
> > code
> > in case we implement rcu_is_cpu_rrupt_from_idle differently such that it
> > does
> > not need the counters but NOHZ_FULL may take issue with that since it needs
> > rcu_user_enter->rcu_eqs_enter to convey that the CPU is "RCU"-idle.
>
> I haven't gone through it in detail, but it seems like we should be able
> to treat in-kernel process-level execution like an interrupt from idle
> or userspace, as the case might be. If we did that, shouldn't we be
> able to just do this?
>
> static int rcu_is_cpu_rrupt_from_idle(void)
> {
> return __this_cpu_read(rcu_data.dynticks_nmi_nesting) <= 1;
> }
I think that would work only if this function is always called from an
interrupt:
The comments on this function says:
* If the current CPU is idle or running at a first-level (not nested)
* interrupt from idle, return true. The caller must have at least
* disabled preemption.
*/
According to this comment rcu_is_cpu_rrupt_from_idle can be called from
somewhere other than an interrupt although from code-reading that does not
seem to be.
If it is ever possible to call rcu_is_cpu_rrupt_from_idle from anywhere but
an interrupt, then we would have a
Re: [PATCH 1/2] i2c: i2c-designware-platdrv: Allow a dynamic adap. nr without an ACPI fwnode
On Tue, Mar 12, 2019 at 04:47:48PM +0200, Jarkko Nikula wrote:
> On 3/11/19 1:22 PM, Hans de Goede wrote:
> > Before this commit the i2c-designware-platdrv assumes that if the pdev
> > has an apci-companion it should use a dynamic adapter-nr and otherwise
> > it will use pdev->id as adapter-nr.
> >
> > On some devices e.g. the Apollo Lake using Acer TravelMate Spin B118,
> > some of the LPSS i2c-adapters are enumerated through PCI and do not have
> > an ACPI fwnode. These devices are handled as mfd devices so they end up
> > using the i2c-designware-platdrv driver.
> >
> > This results in the i2c-adapter being registered with the mfd generated
> > pdev->id as adapter-nr, which conflicts with existing adapters, triggering
> > a WARN(id < 0, "couldn't get idr") in i2c-core-base.c and causing the
> > adapter registration to fail.
> >
> I went thinking would we get a regression if we switch the
> i2c-designware-platdrv to dynamic numbering unconditionally?
>
> Only drivers/mfd/intel-lpss.c and drivers/mfd/intel_quark_i2c_gpio.c
> register platform device "i2c_designware" and otherwise in the driver itself
> for known ACPI IDs and device tree bindings.
>
> Things should be fine for ACPI cases if slave devices are also described in
> ACPI tables. As far as I've understood with device tree matching adapter
> number is irrelevant in slave device registration?
Seems like Hans came to the same conclusion.
> Andy: could you tell by commit 918fe70cf475 ("mfd: intel_quark_i2c_gpio:
> support devices behind i2c bus") are those devices described in ACPI or in
> some i2c_board_infos with referring to fixed adapter number either in or out
> of kernel tree code?
As far as I remember they are coming from ACPI, but you may easily check on
real hardware we have in our lab.
> Then drivers/platform/chrome/chromeos_laptop.c is the only code searching
> for adapter named as "Synopsys DesignWare I2C adapter" without assuming any
> fixed adapter numbering.
> What's unclear to me can there be device tree cases where i2c-designware
> probing comes with pdev->id not starting from zero or in different order?
> I.e. would it make difference do we use pdev->id or dynamic adapter
> numbering?
--
With Best Regards,
Andy Shevchenko
Re: [PATCH v1 1/4] auxdisplay: hd44780: Fix memory leak on ->remove()
Hi Andy,
On Tue, Mar 12, 2019 at 3:40 PM Andy Shevchenko
wrote:
> On Tue, Mar 12, 2019 at 02:47:01PM +0100, Geert Uytterhoeven wrote:
> > On Tue, Mar 12, 2019 at 2:18 PM Andy Shevchenko
> > wrote:
> > > We have to free on ->remove() the allocated resources on ->probe().
> > >
> > > Fixes: d47d88361fee ("auxdisplay: Add HD44780 Character LCD support")
> > > Cc: Geert Uytterhoeven
> > > Signed-off-by: Andy Shevchenko
> >
> > Thanks, nice catch!
> >
> > (wearing a newer (and hopefully wiser ;-) hat than when I wrote the code)
> >
> > While this is correct for the current implementation of struct charlcd_priv,
> > this may be a bit fragile.
>
> This patch will be the same in the next version due to possibility to easy
> backport.
OK.
> > What about adding a charlcd_free() wrapper, which can do
> > kfree(to_priv(lcd)),
> > and be used in the probe failure path, too?
>
> This has been partially done in the rest of the series, but I got your advise
> and will change to use to_priv() in v2. Thanks!
Ah, hadn't noticed the rest of the series. Will review...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v3] zcrypt: handle AP Info notification from CHSC SEI command
On 3/11/19 11:57 AM, Sebastian Ott wrote: On Mon, 11 Mar 2019, Tony Krowiak wrote: On 2/18/19 12:01 PM, Tony Krowiak wrote: The current AP bus implementation periodically polls the AP configuration to detect changes. When the AP configuration is dynamically changed via the SE or an SCLP instruction, the changes will not be reflected to sysfs until the next time the AP configuration is polled. The CHSC architecture provides a Store Event Information (SEI) command to make notification of an AP configuration change. This patch introduces a handler to process notification from the CHSC SEI command by immediately kicking off an AP bus scan-after-event. Ping Applied. Thank you Sebastian.
Re: [PATCH] kernel/trace/trace_kprobe.c - fix comment format
On Tue, 12 Mar 2019 04:58:32 -0400
"Valdis Klētnieks" wrote:
> CC kernel/trace/trace_kprobe.o
> kernel/trace/trace_kprobe.c:41: warning: cannot understand function
> prototype: 'struct trace_kprobe '
>
> The real problem is that a comment looked like kerneldoc when it shouldn't
> be...
>
> Signed-off-by: Valdis Kletnieks
>
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 9eaf07f99212..c825c591553a 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -35,7 +35,7 @@ static struct dyn_event_operations trace_kprobe_ops = {
> .match = trace_kprobe_match,
> };
>
> -/**
> +/*
NACK.
This isn't a kernel doc comment, and I don't want to make it one when
it is not. Just ignore that warning.
-- Steve
> * Kprobe event core functions
> */
> struct trace_kprobe {
Re: [PATCH] infiniband: i40iw: fix potential NULL pointer dereferences
On Fri, Mar 08, 2019 at 11:27:50PM -0600, Kangjie Lu wrote:
> alloc_ordered_workqueue may fail and return NULL. Let's check
> its return value to ensure it is not NULL so as to avoid
> potential NULL pointer dereferences.
>
> Signed-off-by: Kangjie Lu
> drivers/infiniband/hw/i40iw/i40iw_cm.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_cm.c
> b/drivers/infiniband/hw/i40iw/i40iw_cm.c
> index 206cfb0016f8..ad9b4f235e30 100644
> +++ b/drivers/infiniband/hw/i40iw/i40iw_cm.c
> @@ -3256,9 +3256,21 @@ void i40iw_setup_cm_core(struct i40iw_device *iwdev)
>
> cm_core->event_wq = alloc_ordered_workqueue("iwewq",
> WQ_MEM_RECLAIM);
> + if (!cm_core->event_wq) {
> + i40iw_debug(cm_core->dev,
> + I40IW_DEBUG_CM,
> + "%s allocate event work queue failed\n",
> + __func__);
> + }
>
> cm_core->disconn_wq = alloc_ordered_workqueue("iwdwq",
> WQ_MEM_RECLAIM);
> + if (!cm_core->disconn_wq) {
> + i40iw_debug(cm_core->dev,
> + I40IW_DEBUG_CM,
> + "%s allocate disconnect work queue failed\n",
> + __func__);
> + }
> }
Same no as the mlx patches - handle the error or don't bother
Jason
Re: [PATCH] kernel/trace/trace_probe.c - make variable static
On Tue, 12 Mar 2019 04:52:58 -0400
"Valdis Klētnieks" wrote:
> sparse complains:
> CHECK kernel/trace/trace_probe.c
> kernel/trace/trace_probe.c:16:12: warning: symbol 'reserved_field_names' was
> not declared. Should it be static?
>
> Yes, it should be static.
Thanks, applied.
Note, I changed the subject to:
tracing/probes: Make reserved_field_names static
To follow the naming convention of the tracing directory.
-- Steve
>
> Signed-off-by: Valdis Kletnieks
>
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index 89da34b326e3..cfcf77e6fb19 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -13,7 +13,7 @@
>
> #include "trace_probe.h"
>
> -const char *reserved_field_names[] = {
> +static const char *reserved_field_names[] = {
> "common_type",
> "common_flags",
> "common_preempt_count",
Re: [PATCH v9 perf,bpf 08/15] perf, bpf: save btf in a rbtree in perf_env
Em Mon, Mar 11, 2019 at 10:30:44PM -0700, Song Liu escreveu:
> static void perf_env__purge_bpf(struct perf_env *env)
> {
> @@ -83,6 +135,19 @@ static void perf_env__purge_bpf(struct perf_env *env)
> rb_erase(>rb_node, root);
> free(node);
> }
> +
> + root = >bpf_progs.btfs;
> + next = rb_first(root);
> +
> + while (next) {
> + struct btf_node *node;
> +
> + node = rb_entry(next, struct btf_node, rb_node);
> + next = rb_next(>rb_node);
> + rb_erase(>rb_node, root);
> + free(node);
> + }
Added this as well:
env->bpf_progs.btfs_cnt = 0;
> +
> up_write(>bpf_progs.lock);
> }
Re: [PATCH 16/42] drivers: gpio: janz-ttl: drop unneccessary temp variable dev
On 12.03.19 12:26, Thierry Reding wrote: > You're not consistent within the series itself. In patch 3 you went the > other way and dropped usage of pdev->dev in favour of the local dev > variable. ups, you got me :O -- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering i...@metux.net -- +49-151-27565287
[PATCH 3/8] drivers: tty: serial: 8250_ingenic: use devm_ioremap_resource()
Use helper devm_ioremap_resource() to make the code a little bit shorter and easier to read. Signed-off-by: Enrico Weigelt, metux IT consult --- drivers/tty/serial/8250/8250_ingenic.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/tty/serial/8250/8250_ingenic.c b/drivers/tty/serial/8250/8250_ingenic.c index 424c07c..90ae3e2 100644 --- a/drivers/tty/serial/8250/8250_ingenic.c +++ b/drivers/tty/serial/8250/8250_ingenic.c @@ -249,8 +249,7 @@ static int ingenic_uart_probe(struct platform_device *pdev) if (line >= 0) uart.port.line = line; - uart.port.membase = devm_ioremap(>dev, regs->start, -resource_size(regs)); + uart.port.membase = devm_ioremap_resource(>dev, regs); if (!uart.port.membase) return -ENOMEM; -- 1.9.1
[PATCH 8/8] drivers: tty: serial: xilinx_uartps: use helpers
---
drivers/tty/serial/xilinx_uartps.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serial/xilinx_uartps.c
b/drivers/tty/serial/xilinx_uartps.c
index 74089f5..92aff38 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -953,15 +953,14 @@ static int cdns_uart_verify_port(struct uart_port *port,
*/
static int cdns_uart_request_port(struct uart_port *port)
{
- if (!request_mem_region(port->mapbase, CDNS_UART_REGISTER_SPACE,
-CDNS_UART_NAME)) {
+ if (!uart_memres_request(port, CDNS_UART_NAME)) {
return -ENOMEM;
}
- port->membase = ioremap(port->mapbase, CDNS_UART_REGISTER_SPACE);
+ port->membase = uart_memres_ioremap(port);
if (!port->membase) {
dev_err(port->dev, "Unable to map registers\n");
- release_mem_region(port->mapbase, CDNS_UART_REGISTER_SPACE);
+ uart_memres_release(port);
return -ENOMEM;
}
return 0;
@@ -976,7 +975,7 @@ static int cdns_uart_request_port(struct uart_port *port)
*/
static void cdns_uart_release_port(struct uart_port *port)
{
- release_mem_region(port->mapbase, CDNS_UART_REGISTER_SPACE);
+ uart_memres_release(port);
iounmap(port->membase);
port->membase = NULL;
}
@@ -1626,7 +1625,7 @@ static int cdns_uart_probe(struct platform_device *pdev)
* This function also registers this device with the tty layer
* and triggers invocation of the config_port() entry point.
*/
- port->mapbase = res->start;
+ uart_memres_set(*res);
port->irq = irq;
port->dev = >dev;
port->uartclk = clk_get_rate(cdns_uart_data->uartclk);
@@ -1707,7 +1706,7 @@ static int cdns_uart_remove(struct platform_device *pdev)
_uart_data->clk_rate_change_nb);
#endif
rc = uart_remove_one_port(cdns_uart_data->cdns_uart_driver, port);
- port->mapbase = 0;
+ uart_memres_clear(port);
mutex_lock(_lock);
if (cdns_uart_data->id < MAX_UART_INSTANCES)
clear_bit(cdns_uart_data->id, bitmap);
--
1.9.1
[PATCH 1/8] drivers: tty: serial: 8250_bcm2835aux: use devm_platform_ioremap_resource()
---
drivers/tty/serial/8250/8250_bcm2835aux.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_bcm2835aux.c
b/drivers/tty/serial/8250/8250_bcm2835aux.c
index bd53661..0738d14 100644
--- a/drivers/tty/serial/8250/8250_bcm2835aux.c
+++ b/drivers/tty/serial/8250/8250_bcm2835aux.c
@@ -25,7 +25,6 @@ struct bcm2835aux_data {
static int bcm2835aux_serial_probe(struct platform_device *pdev)
{
struct bcm2835aux_data *data;
- struct resource *res;
int ret;
/* allocate the custom structure */
@@ -63,15 +62,12 @@ static int bcm2835aux_serial_probe(struct platform_device
*pdev)
data->uart.port.irq = ret;
/* map the main registers */
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
- dev_err(>dev, "memory resource not found");
- return -EINVAL;
- }
- data->uart.port.membase = devm_ioremap_resource(>dev, res);
+ data->uart.port.membase = devm_platform_ioremap_resource(pdev, 0);
ret = PTR_ERR_OR_ZERO(data->uart.port.membase);
- if (ret)
+ if (ret) {
+ dev_err(>dev, "could not map memory resource");
return ret;
+ }
/* Check for a fixed line number */
ret = of_alias_get_id(pdev->dev.of_node, "serial");
--
1.9.1
[PATCH 6/8] drivers: tty: serial: vt8500: use memres
--- drivers/tty/serial/vt8500_serial.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tty/serial/vt8500_serial.c b/drivers/tty/serial/vt8500_serial.c index 3d58e9b..331a9dd 100644 --- a/drivers/tty/serial/vt8500_serial.c +++ b/drivers/tty/serial/vt8500_serial.c @@ -696,13 +696,13 @@ static int vt8500_serial_probe(struct platform_device *pdev) ); vt8500_port->uart.type = PORT_VT8500; vt8500_port->uart.iotype = UPIO_MEM; - vt8500_port->uart.mapbase = mmres->start; vt8500_port->uart.irq = irqres->start; vt8500_port->uart.fifosize = 16; vt8500_port->uart.ops = _uart_pops; vt8500_port->uart.line = port; vt8500_port->uart.dev = >dev; vt8500_port->uart.flags = UPF_IOREMAP | UPF_BOOT_AUTOCONF; + uart_memres_set(_port->uart, *mmres); /* Serial core uses the magic "16" everywhere - adjust for it */ vt8500_port->uart.uartclk = 16 * clk_get_rate(vt8500_port->clk) / -- 1.9.1
[PATCH 5/8] drivers: tty: serial: introduce struct resource
The standard data structure for holding io resources in the
kernel is struct resource. Serial drivers yet don't really
use it (except when retrieving from oftree).
This patch introduces a new field in struct uart_port for that,
plus several helpers. Yet it's up to the individual drivers for
using it - serial_core doesn't care yet. Therefore, the old fields
mapbase and mapsize need to be filled properly by the drivers.
---
include/linux/serial_core.h | 49 +
1 file changed, 49 insertions(+)
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 5fe2b03..9b07a40 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -24,6 +24,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -256,6 +257,7 @@ struct uart_port {
unsigned intminor;
resource_size_t mapbase;/* for ioremap */
resource_size_t mapsize;
+ struct resource memres;
struct device *dev; /* parent device */
unsigned char hub6; /* this should be in
the 8250 driver */
unsigned char suspended;
@@ -427,6 +429,53 @@ void uart_console_write(struct uart_port *port, const char
*s,
int uart_match_port(struct uart_port *port1, struct uart_port *port2);
/*
+ * Port resource management
+ */
+static inline void uart_memres_set(struct uart_port *port,
+struct resource res)
+{
+ port->memres = res;
+ port->mapbase = res.start;
+}
+
+static inline void uart_memres_clear(struct uart_port *port)
+{
+ port->memres = DEFINE_RES_MEM(0, 0);
+}
+
+static inline int uart_memres_valid(struct uart_port *port)
+{
+ return (port->memres.start != 0);
+}
+
+static inline struct resource *uart_memres_request(struct uart_port *port,
+const char *name)
+{
+ return request_mem_region(
+ port->memres.start,
+ resource_size(>memres),
+ name);
+}
+
+static inline void uart_memres_release(struct uart_port *port)
+{
+ return release_mem_region(port->memres.start,
+ resource_size(>memres));
+}
+
+static inline void __iomem *uart_memres_ioremap_nocache(struct uart_port *port)
+{
+ return ioremap_nocache(port->memres.start,
+ resource_size(>memres.start));
+}
+
+static inline void __iomem *uart_memres_ioremap(struct uart_port *port)
+{
+ return ioremap(port->memres.start,
+ resource_size(>memres.start));
+}
+
+/*
* Power Management
*/
int uart_suspend_port(struct uart_driver *reg, struct uart_port *port);
--
1.9.1
Re: [PATCH] mm/slab: protect cache_reap() against CPU and memory hot plug operations
On Mon 11-03-19 20:17:01, Laurent Dufour wrote:
> The commit 95402b382901 ("cpu-hotplug: replace per-subsystem mutexes with
> get_online_cpus()") remove the CPU_LOCK_ACQUIRE operation which was use to
> grap the cache_chain_mutex lock which was protecting cache_reap() against
> CPU hot plug operations.
>
> Later the commit 18004c5d4084 ("mm, sl[aou]b: Use a common mutex
> definition") changed cache_chain_mutex to slab_mutex but this didn't help
> fixing the missing the cache_reap() protection against CPU hot plug
> operations.
>
> Here we are stopping the per cpu worker while holding the slab_mutex to
> ensure that cache_reap() is not running in our back and will not be
> triggered anymore for this cpu.
>
> This patch fixes that race leading to SLAB's data corruption when CPU
> hotplug are triggered. We hit it while doing partition migration on PowerVM
> leading to CPU reconfiguration through the CPU hotplug mechanism.
What is the actual race? slab_offline_cpu calls cancel_delayed_work_sync
so it removes a pending item and waits for the item to finish if they run
concurently. So why do we need an additional lock?
> This fix is covering kernel containing to the commit 6731d4f12315 ("slab:
> Convert to hotplug state machine"), ie 4.9.1, earlier kernel needs a
> slightly different patch.
>
> Cc: sta...@vger.kernel.org
> Cc: Christoph Lameter
> Cc: Pekka Enberg
> Cc: David Rientjes
> Cc: Joonsoo Kim
> Cc: Andrew Morton
> Signed-off-by: Laurent Dufour
> ---
> mm/slab.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/mm/slab.c b/mm/slab.c
> index 28652e4218e0..ba499d90f27f 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -1103,6 +1103,7 @@ static int slab_online_cpu(unsigned int cpu)
>
> static int slab_offline_cpu(unsigned int cpu)
> {
> + mutex_lock(_mutex);
> /*
>* Shutdown cache reaper. Note that the slab_mutex is held so
>* that if cache_reap() is invoked it cannot do anything
> @@ -1112,6 +1113,7 @@ static int slab_offline_cpu(unsigned int cpu)
> cancel_delayed_work_sync(_cpu(slab_reap_work, cpu));
> /* Now the cache_reaper is guaranteed to be not running. */
> per_cpu(slab_reap_work, cpu).work.func = NULL;
> + mutex_unlock(_mutex);
> return 0;
> }
>
> --
> 2.21.0
--
Michal Hocko
SUSE Labs
RFC: cleaning up the serial drivers and use struct resource
Hello folks, I'm currently working on some cleanups in drivers/tty/serial. There're several cases where new helpers, like devm_platform_ioremap_resource can be used, other places can use devm_ioremap_resource() for a bit cleaner code. Another topic here is using struct resource, instead of separate fields (BTW: struct uart_port->mapsize doesn't seem to be used consequently): in struct uart_port, I'm adding a struct resource field for holding the port's iomem range, and helpers for it. Then, I'm step by step patching the individual drivers to use that, instead of the old fields, directly. For now, this all is early, untested and not yet meant for mainline. I'd just like to have your oppions on my approach. What do you think about it ? thx --mtx
[PATCH 7/8] drivers: tty: serial: use memres
---
drivers/tty/serial/zs.c | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/serial/zs.c b/drivers/tty/serial/zs.c
index b03d3e4..2fd4821 100644
--- a/drivers/tty/serial/zs.c
+++ b/drivers/tty/serial/zs.c
@@ -986,14 +986,13 @@ static void zs_release_port(struct uart_port *uport)
{
iounmap(uport->membase);
uport->membase = 0;
- release_mem_region(uport->mapbase, ZS_CHAN_IO_SIZE);
+ uart_memres_release(uport);
}
static int zs_map_port(struct uart_port *uport)
{
if (!uport->membase)
- uport->membase = ioremap_nocache(uport->mapbase,
-ZS_CHAN_IO_SIZE);
+ uport->membase = uart_memres_ioremap_nocache(uport);
if (!uport->membase) {
printk(KERN_ERR "zs: Cannot map MMIO\n");
return -ENOMEM;
@@ -1005,13 +1004,13 @@ static int zs_request_port(struct uart_port *uport)
{
int ret;
- if (!request_mem_region(uport->mapbase, ZS_CHAN_IO_SIZE, "scc")) {
+ if (!uart_memres_request(uport, "scc")) {
printk(KERN_ERR "zs: Unable to reserve MMIO resource\n");
return -EBUSY;
}
ret = zs_map_port(uport);
if (ret) {
- release_mem_region(uport->mapbase, ZS_CHAN_IO_SIZE);
+ uart_release_memres(uport);
return ret;
}
return 0;
@@ -1113,9 +1112,12 @@ static int __init zs_probe_sccs(void)
uport->flags= UPF_BOOT_AUTOCONF;
uport->ops = _ops;
uport->line = chip * ZS_NUM_CHAN + side;
- uport->mapbase = dec_kn_slot_base +
- zs_parms.scc[chip] +
- (side ^ ZS_CHAN_B) * ZS_CHAN_IO_SIZE;
+ uart_memres_set(
+ DEFINE_RES_MEM(
+ dec_kn_slot_base +
+ zs_parms.scc[chip] +
+ (side ^ ZS_CHAN_B) * ZS_CHAN_IO_SIZE,
+ ZS_CHAN_IO_SIZE));
for (i = 0; i < ZS_NUM_REGS; i++)
zport->regs[i] = zs_init_regs[i];
--
1.9.1
Re: kernel 5.0 blk_clear_pm_only triggers a warning during resume
On Tue, 2019-03-12 at 05:35 +, Jisheng Zhang wrote:
> I got below warning during resume:
>
> [ 673.65] sd 0:0:0:0: [sda] Starting disk
> [ 673.658899] WARNING: CPU: 3 PID: 1039 at blk_clear_pm_only+0x2a/0x30
> [ 673.658902] CPU: 3 PID: 1039 Comm: kworker/u8:49 Not tainted 5.0.0+ #1
> [ 673.658902] sd 2:0:0:0: [sdb] Starting disk
> [ 673.658903] Hardware name: LENOVO 4180F42/4180F42, BIOS 83ET75WW (1.45 )
> 05/10/2013
> [ 673.658906] Workqueue: events_unbound async_run_entry_fn
> [ 673.658909] RIP: 0010:blk_clear_pm_only+0x2a/0x30
> [ 673.658911] Code: b8 ff ff ff ff f0 0f c1 87 80 00 00 00 83 e8 01 78 18 74
> 01 c3 48 81 c7 58 05 00 00 31 c9 31 d2 be 03 00 00 00 e9 36 97 e2 ff <0f> 0b
> c3 0f 1f 00 48 81 c7 90 00 00 00 e9 04 01
> 3a 00 0f 1f 40 00
> [ 673.658911] RSP: :8881115a7e48 EFLAGS: 00010297
> [ 673.658913] RAX: RBX: 888118d42800 RCX:
> 8881194c9a00
> [ 673.658914] RDX: 888118ab1a00 RSI: RDI:
> 888117e7
> [ 673.658915] RBP: 888118d42f90 R08: 0004 R09:
> 0001f900
> [ 673.658915] R10: 45698a8e R11: 0010 R12:
> 888119421000
> [ 673.658916] R13: R14: 88800030ea80 R15:
> 088811942100
> [ 673.658918] FS: () GS:88811a18()
> knlGS:
> [ 673.658918] CS: 0010 DS: ES: CR0: 80050033
> [ 673.658919] CR2: CR3: 0200c001 CR4:
> 000606e0
> [ 673.658920] Call Trace:
> [ 673.658924] ? scsi_device_resume+0x28/0x50
> [ 673.658926] ? scsi_dev_type_resume+0x2b/0x80
> [ 673.658927] ? async_run_entry_fn+0x2c/0xd0
> [ 673.658930] ? process_one_work+0x1f0/0x3f0
> [ 673.658932] ? worker_thread+0x28/0x3c0
> [ 673.658933] ? process_one_work+0x3f0/0x3f0
> [ 673.658935] ? kthread+0x10c/0x130
> [ 673.658936] ? __kthread_create_on_node+0x150/0x150
> [ 673.658938] ? ret_from_fork+0x1f/0x30
> [ 673.658940] ---[ end trace ce18772de33e283e ]---
Hi Jisheng,
Is this something that occurred once or is this something that you can reproduce
easily? In the latter case, can you verify whether the patch below is sufficient
to make this warning disappear?
Thanks,
Bart.
---
drivers/scsi/scsi_lib.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index f3f24dfd8fd5..33e1a72d47fa 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2540,8 +2540,10 @@ void scsi_device_resume(struct scsi_device *sdev)
* device deleted during suspend)
*/
mutex_lock(>state_mutex);
- sdev->quiesced_by = NULL;
- blk_clear_pm_only(sdev->request_queue);
+ if (sdev->quiesced_by) {
+ sdev->quiesced_by = NULL;
+ blk_clear_pm_only(sdev->request_queue);
+ }
if (sdev->sdev_state == SDEV_QUIESCE)
scsi_device_set_state(sdev, SDEV_RUNNING);
mutex_unlock(>state_mutex);
[PATCH 4/8] drivers: tty: serial: use devm_ioremap_resource()
instead of fetching out start and len from a struct resource for
passing it to devm_ioremap(), directly use devm_ioremap_resource()
Signed-off-by: Enrico Weigelt, metux IT consult
---
drivers/tty/serial/8250/8250_lpc18xx.c | 3 +--
drivers/tty/serial/8250/8250_mtk.c | 3 +--
drivers/tty/serial/8250/8250_uniphier.c | 2 +-
drivers/tty/serial/amba-pl010.c | 3 +--
drivers/tty/serial/samsung.c| 2 +-
drivers/tty/serial/sirfsoc_uart.c | 3 +--
6 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_lpc18xx.c
b/drivers/tty/serial/8250/8250_lpc18xx.c
index eddf119..f36902b 100644
--- a/drivers/tty/serial/8250/8250_lpc18xx.c
+++ b/drivers/tty/serial/8250/8250_lpc18xx.c
@@ -119,8 +119,7 @@ static int lpc18xx_serial_probe(struct platform_device
*pdev)
memset(, 0, sizeof(uart));
- uart.port.membase = devm_ioremap(>dev, res->start,
-resource_size(res));
+ uart.port.membase = devm_ioremap_resource(>dev, res);
if (!uart.port.membase)
return -ENOMEM;
diff --git a/drivers/tty/serial/8250/8250_mtk.c
b/drivers/tty/serial/8250/8250_mtk.c
index c1fdbc0..bf6eb8d 100644
--- a/drivers/tty/serial/8250/8250_mtk.c
+++ b/drivers/tty/serial/8250/8250_mtk.c
@@ -383,8 +383,7 @@ static int mtk8250_probe(struct platform_device *pdev)
return -EINVAL;
}
- uart.port.membase = devm_ioremap(>dev, regs->start,
-resource_size(regs));
+ uart.port.membase = devm_ioremap_resource(>dev, regs);
if (!uart.port.membase)
return -ENOMEM;
diff --git a/drivers/tty/serial/8250/8250_uniphier.c
b/drivers/tty/serial/8250/8250_uniphier.c
index 164ba13..9c1244e 100644
--- a/drivers/tty/serial/8250/8250_uniphier.c
+++ b/drivers/tty/serial/8250/8250_uniphier.c
@@ -171,7 +171,7 @@ static int uniphier_uart_probe(struct platform_device *pdev)
return -EINVAL;
}
- membase = devm_ioremap(dev, regs->start, resource_size(regs));
+ membase = devm_ioremap_resource(dev, regs);
if (!membase)
return -ENOMEM;
diff --git a/drivers/tty/serial/amba-pl010.c b/drivers/tty/serial/amba-pl010.c
index 2c37d11..109b8df 100644
--- a/drivers/tty/serial/amba-pl010.c
+++ b/drivers/tty/serial/amba-pl010.c
@@ -713,8 +713,7 @@ static int pl010_probe(struct amba_device *dev, const
struct amba_id *id)
if (!uap)
return -ENOMEM;
- base = devm_ioremap(>dev, dev->res.start,
- resource_size(>res));
+ base = devm_ioremap_resource(>dev, >res);
if (!base)
return -ENOMEM;
diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index 83fd516..a49cf15 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -1775,7 +1775,7 @@ static int s3c24xx_serial_init_port(struct
s3c24xx_uart_port *ourport,
dbg("resource %pR)\n", res);
- port->membase = devm_ioremap(port->dev, res->start, resource_size(res));
+ port->membase = devm_ioremap_resource(port->dev, res);
if (!port->membase) {
dev_err(port->dev, "failed to remap controller address\n");
return -EBUSY;
diff --git a/drivers/tty/serial/sirfsoc_uart.c
b/drivers/tty/serial/sirfsoc_uart.c
index 38622f2..db2e08d 100644
--- a/drivers/tty/serial/sirfsoc_uart.c
+++ b/drivers/tty/serial/sirfsoc_uart.c
@@ -1359,8 +1359,7 @@ static int sirfsoc_uart_probe(struct platform_device
*pdev)
goto err;
}
port->mapbase = res->start;
- port->membase = devm_ioremap(>dev,
- res->start, resource_size(res));
+ port->membase = devm_ioremap_resource(>dev, res);
if (!port->membase) {
dev_err(>dev, "Cannot remap resource.\n");
ret = -ENOMEM;
--
1.9.1
[PATCH 2/8] drivers: tty: serial: 8250_dw: use devm_ioremap_resource()
Use helper devm_ioremap_resource() to make the code a little bit shorter and easier to read. Signed-off-by: Enrico Weigelt, metux IT consult --- drivers/tty/serial/8250/8250_dw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c index d31b975..f0a294d 100644 --- a/drivers/tty/serial/8250/8250_dw.c +++ b/drivers/tty/serial/8250/8250_dw.c @@ -526,7 +526,7 @@ static int dw8250_probe(struct platform_device *pdev) p->set_ldisc= dw8250_set_ldisc; p->set_termios = dw8250_set_termios; - p->membase = devm_ioremap(dev, regs->start, resource_size(regs)); + p->membase = devm_ioremap_resource(dev, regs); if (!p->membase) return -ENOMEM; -- 1.9.1
Re: [PATCH] rcu/tree: Fix self wakeups for grace period kthread
On Tue, 12 Mar 2019 07:09:23 -0700 "Paul E. McKenney" wrote: > On Tue, Mar 12, 2019 at 05:25:28PM +0530, Neeraj Upadhyay wrote: > > On 3/12/19 7:20 AM, Steven Rostedt wrote: > > >On Fri, 8 Mar 2019 15:16:18 +0530 > > >Neeraj Upadhyay wrote: > > > > > >>Update the code to match the comment that self wakeup of > > >>grace period kthread is allowed from interrupt handler, and > > >>softirq handler, running in the grace period kthread's > > >>context. Present code allows self wakeups from all > > >>interrupt contexts - nmi, softirq and hardirq contexts. > > > > > >That's not actually the issue. But it appears that we return if we > > >simply have BH disabled, which I don't think we want, and we don't care > > >about NMI as NMI should never call this code. > > > > > >I think your patch is correct, but the change log is not. > > How about this? > > The current rcu_gp_kthread_wake() function uses in_interrupt() > and thus does a self-wakeup from all interrupt contexts, > including the pointless case where the GP kthread happens to be > running with bottom halves disabled, along with the impossible > case where the GP kthread is running within an NMI handler (you > are not supposed to invoke rcu_gp_kthread_wake() from within an > NMI handler. This commit therefore replaces the in_interrupt() > with in_irq(), so that the self-wakeups happen only from handlers > for hardware interrupts and softirqs. This also makes the code > match the comment. Acked-by: Steven Rostedt (VMware) > > Thanx, Paul > > > >-- Steve > > > > > > > Hi Steve, sorry, I don't understand fully, why we want to not return > > in BH disabled case. From the commit logs and lkml discussion, there > > is a case where GP kthread is interrupted in the wait event path and > > rcu_gp_kthread_wake() is called in softirq handler (I am not sure > > about interrupt handler case; how rcu_gp_kthread_wake() is called > > from that path). BH disabled case isn't a case where the kthread is preempted. It's just that the kthread disabled BH, and thus we want to return. -- Steve
Re: [PATCH v3 1/4] dt: lm3532: Add lm3532 dt doc and update ti_lmu doc
On Tue, Mar 12, 2019 at 07:18:19AM -0500, Dan Murphy wrote: > Add the lm3532 device tree documentation. > Remove lm3532 device tree reference from the ti_lmu devicetree > documentation. > > With the addition of the dedicated lm3532 documentation the device > can be removed from the ti_lmu.txt. > > The reason for this is that the lm3532 dt documentation now defines > the ability to control LED output strings against different control > banks or groups multiple strings to be controlled by a single control > bank. > > Another addition was for ALS lighting control and configuration. The > LM3532 has a feature that can take in the ALS reading from 2 separate > ALS devices and adjust the brightness on the strings that are configured > to support this feature. > > Finally the device specific properties were moved to the parent node as these > properties are not control bank configurable. These include the runtime ramp > and the ALS configuration. > > Signed-off-by: Dan Murphy > --- > > v3 - No changes - https://lore.kernel.org/patchwork/patch/1049026/ > > v2 - Fixed ramp-up and ramp-down properties, removed hard coded property > values, > added ranges for variable properties, I did not change the label - > https://lore.kernel.org/patchwork/patch/1048805/ > > .../devicetree/bindings/leds/leds-lm3532.txt | 127 ++ > .../devicetree/bindings/mfd/ti-lmu.txt| 20 --- > 2 files changed, 127 insertions(+), 20 deletions(-) > create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3532.txt > > diff --git a/Documentation/devicetree/bindings/leds/leds-lm3532.txt > b/Documentation/devicetree/bindings/leds/leds-lm3532.txt > new file mode 100644 > index ..b267d696b511 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/leds-lm3532.txt > @@ -0,0 +1,127 @@ > +* Texas Instruments - lm3532 White LED driver with ambient light sensing > +capability. > + > +The LM3532 provides the 3 high-voltage, low-side current sinks. The device is > +programmable over an I2C-compatible interface and has independent > +current control for all three channels. The adaptive current regulation > +method allows for different LED currents in each current sink thus allowing > +for a wide variety of backlight and keypad applications. > + > +The main features of the LM3532 include dual ambient light sensor inputs > +each with 32 internal voltage setting resistors, 8-bit logarithmic and linear > +brightness control, dual external PWM brightness control inputs, and up to > +1000:1 dimming ratio with programmable fade in and fade out settings. > + > +Required properties: > + - compatible : "ti,lm3532" > + - reg : I2C slave address > + - #address-cells : 1 > + - #size-cells : 0 > + > +Required child properties: > + - reg : Indicates control bank the LED string is controlled by > + - led-sources : see Documentation/devicetree/bindings/leds/common.txt > + - ti,led-mode : Defines if the LED strings are manually controlled or > + if the LED strings are controlled by the ALS. > + 0x00 - LED strings are I2C controlled via full scale > +brightness control register > + 0x01 - LED strings are ALS controlled > + > +Optional child properties: > + Range for ramp settings: 8us - 65536us > + - ramp-up-us - The Run time ramp rates/step are from one current > +set-point to another after the device has reached its > +initial target set point from turn-on > + - ramp-down-us - The Run time ramp rates/step are from one current > + set-point to another after the device has reached its > + initial target set point from turn-on > + > +Optional child properties if ALS mode is used: I think you want to remove 'child' here. Is this all als properties or none or any combination are valid? > + - als-vmin - Minimum ALS voltage defined in Volts > + - als-vmax - Maximum ALS voltage defined in Volts > + Per the data sheet the max ALS voltage is 2V and the min is 0V > + > + - als1-imp-sel - ALS1 impedance resistor selection in Ohms > + - als2-imp-sel - ALS2 impedance resistor selection in Ohms > + Range for impedance select: 37000 Ohms - 1190 Ohms > + Values above 37kohms will be set to the "High Impedance" setting > + > + - als-avrg-time-us - Determines the length of time the device needs to > + average the two ALS inputs. This is only used if > + the input mode is LM3532_ALS_INPUT_AVRG. > + Range: 17920us - 2293760us > + - als-input-mode - Determines how the device uses the attached ALS > +devices. > +0x00 - ALS1 and ALS2 input average > +0x01 - ALS1 Input > +0x02 - ALS2 Input > +0x03 -
Re: [PATCH] tpm: Make timeout logic simpler and more robust
On Mon, Mar 11, 2019 at 04:54:04PM -0700, Calvin Owens wrote: > We're having lots of problems with TPM commands timing out, and we're > seeing these problems across lots of different hardware (both v1/v2). > > I instrumented the driver to collect latency data, but I wasn't able to > find any specific timeout to fix: it seems like many of them are too > aggressive. So I tried replacing all the timeout logic with a single > universal long timeout, and found that makes our TPMs 100% reliable. > > Given that this timeout logic is very complex, problematic, and appears > to serve no real purpose, I propose simply deleting all of it. > > Signed-off-by: Calvin Owens In the current form the commit is doing way too much. I would like to consider duration (command-response latency) and timeouts (state change latency) separately. /Jarkko
Re: [PATCH v9 1/9] bitops: Introduce the for_each_set_clump8 macro
On Tue, Mar 12, 2019 at 04:22:22PM +0900, William Breathitt Gray wrote: > Since Andy appears to have hardware outside of the GPIO subsystem he's > testing, let's wait for that and see how it turns out. Since I have still not much time, here is the driver I'm talking about drivers/thermal/intel/intel_soc_dts_iosf.c If you have a chance to look at it (add_dts_thermal_zone(), for example) and prepare a patch, I will be able to test it on real hardware. -- With Best Regards, Andy Shevchenko
Re: [PATCH 1/2] i2c: i2c-designware-platdrv: Allow a dynamic adap. nr without an ACPI fwnode
Hi,
On 12-03-19 15:47, Jarkko Nikula wrote:
Hi
On 3/11/19 1:22 PM, Hans de Goede wrote:
Before this commit the i2c-designware-platdrv assumes that if the pdev
has an apci-companion it should use a dynamic adapter-nr and otherwise
it will use pdev->id as adapter-nr.
On some devices e.g. the Apollo Lake using Acer TravelMate Spin B118,
some of the LPSS i2c-adapters are enumerated through PCI and do not have
an ACPI fwnode. These devices are handled as mfd devices so they end up
using the i2c-designware-platdrv driver.
This results in the i2c-adapter being registered with the mfd generated
pdev->id as adapter-nr, which conflicts with existing adapters, triggering
a WARN(id < 0, "couldn't get idr") in i2c-core-base.c and causing the
adapter registration to fail.
I went thinking would we get a regression if we switch the
i2c-designware-platdrv to dynamic numbering unconditionally?
Only drivers/mfd/intel-lpss.c and drivers/mfd/intel_quark_i2c_gpio.c register platform
device "i2c_designware" and otherwise in the driver itself for known ACPI IDs
and device tree bindings.
Things should be fine for ACPI cases if slave devices are also described in
ACPI tables. As far as I've understood with device tree matching adapter number
is irrelevant in slave device registration?
Andy: could you tell by commit 918fe70cf475 ("mfd: intel_quark_i2c_gpio: support
devices behind i2c bus") are those devices described in ACPI or in some
i2c_board_infos with referring to fixed adapter number either in or out of kernel tree
code?
Then drivers/platform/chrome/chromeos_laptop.c is the only code searching for adapter
named as "Synopsys DesignWare I2C adapter" without assuming any fixed adapter
numbering.
What's unclear to me can there be device tree cases where i2c-designware probing
comes with pdev->id not starting from zero or in different order? I.e. would it
make difference do we use pdev->id or dynamic adapter numbering?
I've just completed analyzing all ways a designware_i2c (or compatible)
platform device can be instantiated and I've come to the conclusion that
always using dynamic adapter-nrs is fine and is a proper, clean fix for this.
Also see my reply to Andy which I send about the same time you send
your reply :)
Regards,
Hans
Re: [PATCH 1/2] i2c: i2c-designware-platdrv: Allow a dynamic adap. nr without an ACPI fwnode
Hi, On 11-03-19 13:52, Andy Shevchenko wrote: On Mon, Mar 11, 2019 at 12:22:15PM +0100, Hans de Goede wrote: Before this commit the i2c-designware-platdrv assumes that if the pdev has an apci-companion it should use a dynamic adapter-nr and otherwise it will use pdev->id as adapter-nr. On some devices e.g. the Apollo Lake using Acer TravelMate Spin B118, some of the LPSS i2c-adapters are enumerated through PCI and do not have an ACPI fwnode. These devices are handled as mfd devices so they end up using the i2c-designware-platdrv driver. This results in the i2c-adapter being registered with the mfd generated pdev->id as adapter-nr, which conflicts with existing adapters, triggering a WARN(id < 0, "couldn't get idr") in i2c-core-base.c and causing the adapter registration to fail. This commit adds support for setting a "linux,use-dynamic-adapter-nr" device property on the device to make i2c-designware-platdrv use dynamic adapter-nrs on devices without an ACPI fwnode, together with changes to drivers/mfd/intel-lpss-pci.c to set this, this fixes the WARN. Before this commit the setting of the adapter.nr was somewhat convoluted, in the acpi_companion case it was set from dw_i2c_acpi_configure, in the non acpi_companion case it was set from dw_i2c_set_fifo_size() based on tx_fifo_depth not being set yet. This commit also cleans this up. Can we split this to two patches, i.e. one is almost the same as this one, except the second one adds a new property check to the conditional? Ok, I've split the patch for v2 of the series. If you agree to do so, you may add mine Rb tag to the first one out of three. Note the "linux,use-dynamic-adapter-nr" is meant for kernel internal use only, therefor it is NOT documented under Documents/devicetree/bindings. To the second and third ones, can we rather check if the device has fwnode either ACPI or swnode? AFAIU now we have swnode assigned during MFD device registration and can easily distinguish this w/o any additional properties. Detecting a swnode is non trivial, it may be either the primary or secondary fwnode depending if the device already had a node before calling device_add_properties. More importantly deciding this based on the presence of a swnode feels like its "wrong". But I've come up with a better solution. I've analyzed all ways a designware_i2c compatible platform-device can be instantiated and all cases except for: drivers/mfd/intel_quark_i2c_gpio.c drivers/mfd/intel-lpss-pci.c Are always using dynamic adapter numbers already. The Quark X1000 has only 1 i2c controller, so there using dynamic adapter numbers will not make a difference. And the intel-lpss-pci.c case is the one which we actually want to fix, devices using this code have many i2c busses so using a fixed bus number leads to the bus-number conflicts this patch is trying to fix. So we can simply always use dynamic adapter numbers, see the commit message of the second patch in v2 of this series for the full analysis. Regards, Hans
Re: [PATCH v2 1/9] mfd: mt6397: clean up code
On 11/03/2019 20:01, Sean Wang wrote:
> Hi,
>
> On Sun, Mar 10, 2019 at 8:48 PM Hsin-Hsiung Wang
> wrote:
>>
>> clean up code
>>
>> Signed-off-by: Hsin-Hsiung Wang
>> ---
>> drivers/mfd/mt6397-core.c | 16
>> 1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c
>> index 77b64bd..acb9812 100644
>> --- a/drivers/mfd/mt6397-core.c
>> +++ b/drivers/mfd/mt6397-core.c
>> @@ -18,17 +18,17 @@
>> #include
>> #include
>> #include
>> -#include
>> #include
>> -#include
>> +#include
>> #include
>> +#include
>>
>> #define MT6397_RTC_BASE0xe000
>> #define MT6397_RTC_SIZE0x3e
>>
>> -#define MT6323_CID_CODE0x23
>> -#define MT6391_CID_CODE0x91
>> -#define MT6397_CID_CODE0x97
>> +#define MT6323_CHIP_ID 0x23
>> +#define MT6391_CHIP_ID 0x91
>> +#define MT6397_CHIP_ID 0x97
>>
>
> It would be not necessary to simply rename the definition or do you
> have a strong reason to do that?
>
I agree, apart, please provide a sound commit message.
"clean up code" is difficult to understand.
Regards,
Matthias
>> static const struct resource mt6397_rtc_resources[] = {
>> {
>> @@ -298,7 +298,7 @@ static int mt6397_probe(struct platform_device *pdev)
>> return pmic->irq;
>>
>> switch (id & 0xff) {
>> - case MT6323_CID_CODE:
>> + case MT6323_CHIP_ID:
>> pmic->int_con[0] = MT6323_INT_CON0;
>> pmic->int_con[1] = MT6323_INT_CON1;
>> pmic->int_status[0] = MT6323_INT_STATUS0;
>> @@ -312,8 +312,8 @@ static int mt6397_probe(struct platform_device *pdev)
>>0, pmic->irq_domain);
>> break;
>>
>> - case MT6397_CID_CODE:
>> - case MT6391_CID_CODE:
>> + case MT6391_CHIP_ID:
>> + case MT6397_CHIP_ID:
>> pmic->int_con[0] = MT6397_INT_CON0;
>> pmic->int_con[1] = MT6397_INT_CON1;
>> pmic->int_status[0] = MT6397_INT_STATUS0;
>> --
>> 1.9.1
>>
>>
>> ___
>> Linux-mediatek mailing list
>> linux-media...@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-mediatek
Re: [PATCH 1/2] i2c: i2c-designware-platdrv: Allow a dynamic adap. nr without an ACPI fwnode
Hi
On 3/11/19 1:22 PM, Hans de Goede wrote:
Before this commit the i2c-designware-platdrv assumes that if the pdev
has an apci-companion it should use a dynamic adapter-nr and otherwise
it will use pdev->id as adapter-nr.
On some devices e.g. the Apollo Lake using Acer TravelMate Spin B118,
some of the LPSS i2c-adapters are enumerated through PCI and do not have
an ACPI fwnode. These devices are handled as mfd devices so they end up
using the i2c-designware-platdrv driver.
This results in the i2c-adapter being registered with the mfd generated
pdev->id as adapter-nr, which conflicts with existing adapters, triggering
a WARN(id < 0, "couldn't get idr") in i2c-core-base.c and causing the
adapter registration to fail.
I went thinking would we get a regression if we switch the
i2c-designware-platdrv to dynamic numbering unconditionally?
Only drivers/mfd/intel-lpss.c and drivers/mfd/intel_quark_i2c_gpio.c
register platform device "i2c_designware" and otherwise in the driver
itself for known ACPI IDs and device tree bindings.
Things should be fine for ACPI cases if slave devices are also described
in ACPI tables. As far as I've understood with device tree matching
adapter number is irrelevant in slave device registration?
Andy: could you tell by commit 918fe70cf475 ("mfd: intel_quark_i2c_gpio:
support devices behind i2c bus") are those devices described in ACPI or
in some i2c_board_infos with referring to fixed adapter number either in
or out of kernel tree code?
Then drivers/platform/chrome/chromeos_laptop.c is the only code
searching for adapter named as "Synopsys DesignWare I2C adapter" without
assuming any fixed adapter numbering.
What's unclear to me can there be device tree cases where i2c-designware
probing comes with pdev->id not starting from zero or in different
order? I.e. would it make difference do we use pdev->id or dynamic
adapter numbering?
--
Jarkko
Re: [PATCH v2 2/3] dt-bindings: mmc: Add a new property disable-cqe-dcmd.
> On 12.03.2019, at 14:17, Heiko Stuebner wrote: > > Am Freitag, 8. März 2019, 14:10:45 CET schrieb Christoph Müllner: >> >>> On 08.03.2019, at 13:46, Adrian Hunter wrote: >>> >>> On 7/03/19 10:43 AM, Christoph Muellner wrote: This patch documents the new property disable-cqe-dcmd for the Arasan eMMC 5.1 driver. Signed-off-by: Christoph Muellner Signed-off-by: Philipp Tomsich --- Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt index 1edbb049cccb..ec699bf98b7c 100644 --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt @@ -44,6 +44,10 @@ Optional Properties: properly. Test mode can be used to force the controller to function. - xlnx,int-clock-stable-broken: when present, the controller always reports that the internal clock is stable even when it is not. + - disable-cqe-dcmd: The eMMC 5.1 standard specifies direct commands (DCMDs) +as part of the command queue engine (CQE). On controllers with a CQHCI, +such as the Arasan eMMC 5.1 host controller, the driver has to enable DCMDs. +This is done unless disable-cqe-dcmd is specified. > > This needs a rewording please. See below for hw-description vs. driver, so > the description should be centered around why this is a property of the hw > [like faulty controller implementation or whatever] I understand, that you prefer a HW-specific name for a property over one, that explains the actual effect. However, using disable- (which is always a directive for the driver and not a HW description) is not uncommon: * disable-wp should be "no-wp-line-connection" * disable-over-current -> "no-over-current-line-connection" * srp-disable -> "not-existing-srp-implementation" * ... But I don't mind using something else. Would "broken-cqe-dcmd" (like broken-cd or broken-flash-reset) be ok? Or other suggestions? Also I'd like to mention, that my first implementation was "supports-cqe-dcmd". I guess that would be a good choice, if it would have been there from the beginning. Introducing it now, would silently disable DCMD for existing DTBs. Therefore I went on to "disable-cqe-dcmd". > > >>> If "supports-cqe" is in mmc.txt, should "disable-cqe-dcmd" be there also? >> >> The file mmc.txt says on top: >> "These properties are common to multiple MMC host controllers". >> As my patchset introduces "disable-cqe-dcmd" just for sdhci-of-arasan, >> I would say it should not go into that file. >> >> Also I wonder why "supports-cqe" is in mmc.txt, because >> only sdhci-tegra.c is evaluating that property. >> So I would expect it to be documented in >> Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt >> >> However, I see that "disable-cqe-dcmd" could go into other drivers as well. >> But is this enough to document it in mmc.txt? > > Devicetree is always about describing the hardware capabilites and never > about the actual nitty-gritty of driver implementation, aka it is not meant > as a space for hardware-independent config-settings or such. > > As for only tegra evaluating this, is probably because it is still so new, > like > january 2019 and Rob explicitly suggested it becoming common [0], which > suggests that the disable-cqe-dcmds should probably also be common. So mmc.txt lists "standardised" names for properties to reduce the risk of having similar, but distinct names for different MMC drivers. So whenever you want to introduce a new property for a driver, check if there isn't already something defined in mmc.txt. When seeing it this way, it clearly makes sense to have the property there. Thanks, Christoph
Re: KASAN: null-ptr-deref Read in reclaim_high
On Tue, Mar 12, 2019 at 2:46 PM Shakeel Butt wrote:
>
> On Tue, Mar 12, 2019 at 1:33 AM Dmitry Vyukov wrote:
> >
> > On Tue, Mar 12, 2019 at 7:25 AM Andrew Morton
> > wrote:
> > >
> > > On Tue, 12 Mar 2019 07:08:38 +0100 Dmitry Vyukov
> > > wrote:
> > >
> > > > On Tue, Mar 12, 2019 at 12:37 AM Andrew Morton
> > > > wrote:
> > > > >
> > > > > On Mon, 11 Mar 2019 06:08:01 -0700 syzbot
> > > > > wrote:
> > > > >
> > > > > > syzbot has bisected this bug to:
> > > > > >
> > > > > > commit 29a4b8e275d1f10c51c7891362877ef6cffae9e7
> > > > > > Author: Shakeel Butt
> > > > > > Date: Wed Jan 9 22:02:21 2019 +
> > > > > >
> > > > > > memcg: schedule high reclaim for remote memcgs on high_work
> > > > > >
> > > > > > bisection log:
> > > > > > https://syzkaller.appspot.com/x/bisect.txt?x=155bf5db20
> > > > > > start commit: 29a4b8e2 memcg: schedule high reclaim for remote
> > > > > > memcgs on..
> > > > > > git tree: linux-next
> > > > > > final crash:
> > > > > > https://syzkaller.appspot.com/x/report.txt?x=175bf5db20
> > > > > > console output:
> > > > > > https://syzkaller.appspot.com/x/log.txt?x=135bf5db20
> > > > > > kernel config:
> > > > > > https://syzkaller.appspot.com/x/.config?x=611f89e5b6868db
> > > > > > dashboard link:
> > > > > > https://syzkaller.appspot.com/bug?extid=fa11f9da42b46cea3b4a
> > > > > > userspace arch: amd64
> > > > > > syz repro:
> > > > > > https://syzkaller.appspot.com/x/repro.syz?x=1425901740
> > > > > > C reproducer:
> > > > > > https://syzkaller.appspot.com/x/repro.c?x=141630a0c0
> > > > > >
> > > > > > Reported-by: syzbot+fa11f9da42b46cea3...@syzkaller.appspotmail.com
> > > > > > Fixes: 29a4b8e2 ("memcg: schedule high reclaim for remote memcgs on
> > > > > > high_work")
> > > > >
> > > > > The following patch
> > > > > memcg-schedule-high-reclaim-for-remote-memcgs-on-high_work-v3.patch
> > > > > might have fixed this. Was it applied?
> > > >
> > > > Hi Andrew,
> > > >
> > > > You mean if the patch was applied during the bisection?
> > > > No, it wasn't. Bisection is very specifically done on the same tree
> > > > where the bug was hit. There are already too many factors that make
> > > > the result flaky/wrong/inconclusive without changing the tree state.
> > > > Now, if syzbot would know about any pending fix for this bug, then it
> > > > would not do the bisection at all. But it have not seen any patch in
> > > > upstream/linux-next with the Reported-by tag, nor it received any syz
> > > > fix commands for this bugs. Should have been it aware of the fix? How?
> > >
> > > memcg-schedule-high-reclaim-for-remote-memcgs-on-high_work-v3.patch was
> > > added to linux-next on Jan 10. I take it that this bug was hit when
> > > testing the entire linux-next tree, so we can assume that
> > > memcg-schedule-high-reclaim-for-remote-memcgs-on-high_work-v3.patch
> > > does not fix it, correct?
> > > In which case, over to Shakeel!
> >
> > Jan 10 is exactly when this bug was reported:
> > https://groups.google.com/forum/#!msg/syzkaller-bugs/5YkhNUg2PFY/4-B5M7bDCAAJ
> > https://syzkaller.appspot.com/bug?extid=fa11f9da42b46cea3b4a
> >
> > We don't know if that patch fixed the bug or not because nobody tested
> > the reproducer with that patch.
> >
> > It seems that the problem here is that nobody associated the fix with
> > the bug report. So people looking at open bug reports will spend time
> > again and again debugging this just to find that this was fixed months
> > ago. syzbot also doesn't have a chance to realize that this is fixed
> > and bisection is not necessary anymore. It also won't confirm/disprove
> > that the fix actually fixes the bug because even if the crash will
> > continue to happen it will look like the old crash just continues to
> > happen, so nothing to notify about.
> >
> > Associating fixes with bug reports solves all these problems for
> > humans and bots.
>
> Should we add "Reported-by" for syzbot reports on linux-next patches
> as well? Please note that these patches are in flux and might be
> dropped or completely changed before merging into Linus tree.
Reported-by will work, but may be confusing. It was discussed that for
squashed fixed Tested-by tag may be more appropriate and will also
work.
For dropped patches we don't have a better way other than marking it
as invalid for now.
> Also should syzbot drop bug reports on older linux-next trees if it
> can not be repro'ed in the latest linux-next tree? IMHO yes.
Please file an issue for this at:
https://github.com/google/syzkaller/issues
So far we don't have retesting in any form. Some bugs don't have
repros, so can't be retested. Any closing should probably happen after
long enough timeout to avoid spam, so bisection for them will most
likely happen anyway.
I can't promise any ETA for linux-next-specific work. Testing of
linux-next happens mostly because it's done on a rules not
significantly different from everything else (other trees,
[PATCH v2 0/5] auxdisplay: Introduce charlcd_free()
I have found a memory leak in hd44780 and it becomes that we have no counterpart to charlcd_alloc() that developers can easily miss. So, this series fixes a leak and introduces the charlcd_free(). In v2: - add new patch to convert to_priv() to charlcd_to_priv() - address Geert's comment what should be freed Cc: Geert Uytterhoeven Andy Shevchenko (5): auxdisplay: hd44780: Fix memory leak on ->remove() auxdisplay: charlcd: Move to_priv() to charlcd namespace auxdisplay: charlcd: Introduce charlcd_free() helper auxdisplay: panel: Convert to use charlcd_free() auxdisplay: hd44780: Convert to use charlcd_free() drivers/auxdisplay/charlcd.c | 32 +++- drivers/auxdisplay/hd44780.c | 4 +++- drivers/auxdisplay/panel.c | 4 ++-- include/misc/charlcd.h | 1 + 4 files changed, 25 insertions(+), 16 deletions(-) -- 2.20.1
[PATCH v2 4/5] auxdisplay: panel: Convert to use charlcd_free()
Convert to use charlcd_free() instead of kfree() for sake of type check.
Signed-off-by: Andy Shevchenko
---
drivers/auxdisplay/panel.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/auxdisplay/panel.c b/drivers/auxdisplay/panel.c
index 21b9b2f2470a..e06de63497cf 100644
--- a/drivers/auxdisplay/panel.c
+++ b/drivers/auxdisplay/panel.c
@@ -1620,7 +1620,7 @@ static void panel_attach(struct parport *port)
if (lcd.enabled)
charlcd_unregister(lcd.charlcd);
err_unreg_device:
- kfree(lcd.charlcd);
+ charlcd_free(lcd.charlcd);
lcd.charlcd = NULL;
parport_unregister_device(pprt);
pprt = NULL;
@@ -1647,7 +1647,7 @@ static void panel_detach(struct parport *port)
if (lcd.enabled) {
charlcd_unregister(lcd.charlcd);
lcd.initialized = false;
- kfree(lcd.charlcd);
+ charlcd_free(lcd.charlcd);
lcd.charlcd = NULL;
}
--
2.20.1
[PATCH v2 1/5] auxdisplay: hd44780: Fix memory leak on ->remove()
We have to free on ->remove() the allocated resources on ->probe().
Fixes: d47d88361fee ("auxdisplay: Add HD44780 Character LCD support")
Cc: Geert Uytterhoeven
Signed-off-by: Andy Shevchenko
---
drivers/auxdisplay/hd44780.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/auxdisplay/hd44780.c b/drivers/auxdisplay/hd44780.c
index 9ad93ea42fdc..3cde351fb5c9 100644
--- a/drivers/auxdisplay/hd44780.c
+++ b/drivers/auxdisplay/hd44780.c
@@ -280,6 +280,8 @@ static int hd44780_remove(struct platform_device *pdev)
struct charlcd *lcd = platform_get_drvdata(pdev);
charlcd_unregister(lcd);
+
+ kfree(lcd);
return 0;
}
--
2.20.1
[PATCH v2 5/5] auxdisplay: hd44780: Convert to use charlcd_free()
Convert to use charlcd_free() instead of kfree() for sake of type check. Signed-off-by: Andy Shevchenko --- drivers/auxdisplay/hd44780.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/auxdisplay/hd44780.c b/drivers/auxdisplay/hd44780.c index 3cde351fb5c9..ab15b64707ad 100644 --- a/drivers/auxdisplay/hd44780.c +++ b/drivers/auxdisplay/hd44780.c @@ -271,7 +271,7 @@ static int hd44780_probe(struct platform_device *pdev) return 0; fail: - kfree(lcd); + charlcd_free(lcd); return ret; } @@ -281,7 +281,7 @@ static int hd44780_remove(struct platform_device *pdev) charlcd_unregister(lcd); - kfree(lcd); + charlcd_free(lcd); return 0; } -- 2.20.1
[PATCH v2 2/5] auxdisplay: charlcd: Move to_priv() to charlcd namespace
In order to be more particular in names, rename to_priv() macro
to charlcd_to_priv().
No functional change intended.
Cc: Geert Uytterhoeven
Signed-off-by: Andy Shevchenko
---
drivers/auxdisplay/charlcd.c | 26 +-
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/auxdisplay/charlcd.c b/drivers/auxdisplay/charlcd.c
index 60e0b772673f..407acd22efa8 100644
--- a/drivers/auxdisplay/charlcd.c
+++ b/drivers/auxdisplay/charlcd.c
@@ -91,7 +91,7 @@ struct charlcd_priv {
unsigned long long drvdata[0];
};
-#define to_priv(p) container_of(p, struct charlcd_priv, lcd)
+#define charlcd_to_priv(p) container_of(p, struct charlcd_priv, lcd)
/* Device single-open policy control */
static atomic_t charlcd_available = ATOMIC_INIT(1);
@@ -105,7 +105,7 @@ static void long_sleep(int ms)
/* turn the backlight on or off */
static void charlcd_backlight(struct charlcd *lcd, int on)
{
- struct charlcd_priv *priv = to_priv(lcd);
+ struct charlcd_priv *priv = charlcd_to_priv(lcd);
if (!lcd->ops->backlight)
return;
@@ -134,7 +134,7 @@ static void charlcd_bl_off(struct work_struct *work)
/* turn the backlight on for a little while */
void charlcd_poke(struct charlcd *lcd)
{
- struct charlcd_priv *priv = to_priv(lcd);
+ struct charlcd_priv *priv = charlcd_to_priv(lcd);
if (!lcd->ops->backlight)
return;
@@ -152,7 +152,7 @@ EXPORT_SYMBOL_GPL(charlcd_poke);
static void charlcd_gotoxy(struct charlcd *lcd)
{
- struct charlcd_priv *priv = to_priv(lcd);
+ struct charlcd_priv *priv = charlcd_to_priv(lcd);
unsigned int addr;
/*
@@ -170,7 +170,7 @@ static void charlcd_gotoxy(struct charlcd *lcd)
static void charlcd_home(struct charlcd *lcd)
{
- struct charlcd_priv *priv = to_priv(lcd);
+ struct charlcd_priv *priv = charlcd_to_priv(lcd);
priv->addr.x = 0;
priv->addr.y = 0;
@@ -179,7 +179,7 @@ static void charlcd_home(struct charlcd *lcd)
static void charlcd_print(struct charlcd *lcd, char c)
{
- struct charlcd_priv *priv = to_priv(lcd);
+ struct charlcd_priv *priv = charlcd_to_priv(lcd);
if (priv->addr.x < lcd->bwidth) {
if (lcd->char_conv)
@@ -211,7 +211,7 @@ static void charlcd_clear_fast(struct charlcd *lcd)
/* clears the display and resets X/Y */
static void charlcd_clear_display(struct charlcd *lcd)
{
- struct charlcd_priv *priv = to_priv(lcd);
+ struct charlcd_priv *priv = charlcd_to_priv(lcd);
lcd->ops->write_cmd(lcd, LCD_CMD_DISPLAY_CLEAR);
priv->addr.x = 0;
@@ -223,7 +223,7 @@ static void charlcd_clear_display(struct charlcd *lcd)
static int charlcd_init_display(struct charlcd *lcd)
{
void (*write_cmd_raw)(struct charlcd *lcd, int cmd);
- struct charlcd_priv *priv = to_priv(lcd);
+ struct charlcd_priv *priv = charlcd_to_priv(lcd);
u8 init;
if (lcd->ifwidth != 4 && lcd->ifwidth != 8)
@@ -369,7 +369,7 @@ static bool parse_xy(const char *s, unsigned long *x,
unsigned long *y)
static inline int handle_lcd_special_code(struct charlcd *lcd)
{
- struct charlcd_priv *priv = to_priv(lcd);
+ struct charlcd_priv *priv = charlcd_to_priv(lcd);
/* LCD special codes */
@@ -580,7 +580,7 @@ static inline int handle_lcd_special_code(struct charlcd
*lcd)
static void charlcd_write_char(struct charlcd *lcd, char c)
{
- struct charlcd_priv *priv = to_priv(lcd);
+ struct charlcd_priv *priv = charlcd_to_priv(lcd);
/* first, we'll test if we're in escape mode */
if ((c != '\n') && priv->esc_seq.len >= 0) {
@@ -705,7 +705,7 @@ static ssize_t charlcd_write(struct file *file, const char
__user *buf,
static int charlcd_open(struct inode *inode, struct file *file)
{
- struct charlcd_priv *priv = to_priv(the_charlcd);
+ struct charlcd_priv *priv = charlcd_to_priv(the_charlcd);
int ret;
ret = -EBUSY;
@@ -766,7 +766,7 @@ static void charlcd_puts(struct charlcd *lcd, const char *s)
/* initialize the LCD driver */
static int charlcd_init(struct charlcd *lcd)
{
- struct charlcd_priv *priv = to_priv(lcd);
+ struct charlcd_priv *priv = charlcd_to_priv(lcd);
int ret;
if (lcd->ops->backlight) {
@@ -866,7 +866,7 @@ EXPORT_SYMBOL_GPL(charlcd_register);
int charlcd_unregister(struct charlcd *lcd)
{
- struct charlcd_priv *priv = to_priv(lcd);
+ struct charlcd_priv *priv = charlcd_to_priv(lcd);
unregister_reboot_notifier(_notifier);
charlcd_puts(lcd, "\x0cLCD driver unloaded.\x1b[Lc\x1b[Lb\x1b[L-");
--
2.20.1
[PATCH v2 3/5] auxdisplay: charlcd: Introduce charlcd_free() helper
The charlcd_free() is a counterpart to charlcd_alloc()
and should be called symmetrically on tear down.
Cc: Geert Uytterhoeven
Signed-off-by: Andy Shevchenko
---
drivers/auxdisplay/charlcd.c | 6 ++
include/misc/charlcd.h | 1 +
2 files changed, 7 insertions(+)
diff --git a/drivers/auxdisplay/charlcd.c b/drivers/auxdisplay/charlcd.c
index 407acd22efa8..a351a9400054 100644
--- a/drivers/auxdisplay/charlcd.c
+++ b/drivers/auxdisplay/charlcd.c
@@ -818,6 +818,12 @@ struct charlcd *charlcd_alloc(unsigned int drvdata_size)
}
EXPORT_SYMBOL_GPL(charlcd_alloc);
+void charlcd_free(struct charlcd *lcd)
+{
+ kfree(charlcd_to_priv(lcd));
+}
+EXPORT_SYMBOL_GPL(charlcd_free);
+
static int panel_notify_sys(struct notifier_block *this, unsigned long code,
void *unused)
{
diff --git a/include/misc/charlcd.h b/include/misc/charlcd.h
index 23f61850f363..1832402324ce 100644
--- a/include/misc/charlcd.h
+++ b/include/misc/charlcd.h
@@ -35,6 +35,7 @@ struct charlcd_ops {
};
struct charlcd *charlcd_alloc(unsigned int drvdata_size);
+void charlcd_free(struct charlcd *lcd);
int charlcd_register(struct charlcd *lcd);
int charlcd_unregister(struct charlcd *lcd);
--
2.20.1
Re: [PATCH 4/9] drivers: ata: ahci_xgene: use devm_platform_ioremap_resource()
Hello!
On 03/12/2019 12:19 PM, Enrico Weigelt, metux IT consult wrote:
> Use the new helper that wraps the calls to platform_get_resource()
> and devm_ioremap_resource() together.
>
> Signed-off-by: Enrico Weigelt, metux IT consult
> ---
> drivers/ata/ahci_xgene.c | 21 +++--
> 1 file changed, 7 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
> index 7e157e1..99c622c 100644
> --- a/drivers/ata/ahci_xgene.c
> +++ b/drivers/ata/ahci_xgene.c
> @@ -752,7 +752,6 @@ static int xgene_ahci_probe(struct platform_device *pdev)
[...]
> /* Retrieve the optional IP mux resource */
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 4);
> - if (res) {
> - void __iomem *csr = devm_ioremap_resource(dev, res);
> - if (IS_ERR(csr))
> - return PTR_ERR(csr);
> -
> - ctx->csr_mux = csr;
> + ctx->csr_mux = csr = devm_platform_ioremap_resource(pdev, 4);
> + if (IS_ERR(ctx->csr_mux)) {
> + dev_info(>dev, "cant get ip mux resource (optional)");
Can't.
> + ctx->csr_mux = NULL;
MBR, Sergei
Re: [PATCH] tpm: Make timeout logic simpler and more robust
On Tue, 2019-03-12 at 14:50 +0200, Jarkko Sakkinen wrote: > On Mon, Mar 11, 2019 at 05:27:43PM -0700, James Bottomley wrote: > > On Mon, 2019-03-11 at 16:54 -0700, Calvin Owens wrote: > > > e're having lots of problems with TPM commands timing out, and > > > we're seeing these problems across lots of different hardware > > > (both v1/v2). > > > > > > I instrumented the driver to collect latency data, but I wasn't > > > able to find any specific timeout to fix: it seems like many of > > > them are too aggressive. So I tried replacing all the timeout > > > logic with a single universal long timeout, and found that makes > > > our TPMs 100% reliable. > > > > > > Given that this timeout logic is very complex, problematic, and > > > appears to serve no real purpose, I propose simply deleting all > > > of it. > > > > "no real purpose" is a bit strong given that all these timeouts are > > standards mandated. The purpose stated by the standards is that > > there needs to be a way of differentiating the TPM crashed from the > > TPM is taking a very long time to respond. For a normally > > functioning TPM it looks complex and unnecessary, but for a > > malfunctioning one it's a lifesaver. > > Standards should be only followed when they make practical sense and > ignored when not. The range is only up to 2s anyway. I don't disagree ... and I'm certainly not going to defend the TCG because I do think the complexity of some of its standards contributed to the lack of use of TPM 1.2. However, I am saying we should root cause this problem rather than take a blind shot at the apparent timeout complexity. My timeout instability is definitely related to the polling adjustments, so it's not unreasonable to think Facebooks might be as well. James
Re: [PATCH v8 1/2] dt-bindings: serial: Add compatible for Mediatek MT8183
On 11/03/2019 09:54, Erin Lo wrote: > This adds dt-binding documentation of uart for Mediatek MT8183 SoC > Platform. > > Signed-off-by: Erin Lo > Acked-by: Rob Herring > --- > Documentation/devicetree/bindings/serial/mtk-uart.txt | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/serial/mtk-uart.txt > b/Documentation/devicetree/bindings/serial/mtk-uart.txt > index 742cb47..bcfb131 100644 > --- a/Documentation/devicetree/bindings/serial/mtk-uart.txt > +++ b/Documentation/devicetree/bindings/serial/mtk-uart.txt > @@ -16,6 +16,7 @@ Required properties: >* "mediatek,mt8127-uart" for MT8127 compatible UARTS >* "mediatek,mt8135-uart" for MT8135 compatible UARTS >* "mediatek,mt8173-uart" for MT8173 compatible UARTS > + * "mediatek,mt8183-uart", "mediatek,mt6577-uart" for MT8183 compatible > UARTS >* "mediatek,mt6577-uart" for MT6577 and all of the above > > - reg: The base address of the UART register bank. > Acked-by: Matthias Brugger Greg will you take this through your branch or do you prefer me to queue it? Regards, Matthias
Re: [PATCH v1 1/4] auxdisplay: hd44780: Fix memory leak on ->remove()
On Tue, Mar 12, 2019 at 02:47:01PM +0100, Geert Uytterhoeven wrote:
> On Tue, Mar 12, 2019 at 2:18 PM Andy Shevchenko
> wrote:
> > We have to free on ->remove() the allocated resources on ->probe().
> >
> > Fixes: d47d88361fee ("auxdisplay: Add HD44780 Character LCD support")
> > Cc: Geert Uytterhoeven
> > Signed-off-by: Andy Shevchenko
>
> Thanks, nice catch!
>
> (wearing a newer (and hopefully wiser ;-) hat than when I wrote the code)
>
> While this is correct for the current implementation of struct charlcd_priv,
> this may be a bit fragile.
This patch will be the same in the next version due to possibility to easy
backport.
> What about adding a charlcd_free() wrapper, which can do kfree(to_priv(lcd)),
> and be used in the probe failure path, too?
This has been partially done in the rest of the series, but I got your advise
and will change to use to_priv() in v2. Thanks!
--
With Best Regards,
Andy Shevchenko
[PATCH 2/2] Staging: rtl8723bs: os_dep: Invert if selection statement
From: Guilherme T Maeoka
Change 'if (a)' to 'if (!a)' and return. Otherwise, continue with
the previouly wrapped block of control. This reduces the indentation
level by 2 and 1.
I'm not if this commit contributes to the coding style.
Signed-off-by: Guilherme T Maeoka
---
drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 73 +++-
1 file changed, 40 insertions(+), 33 deletions(-)
diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
index 86f1e090436e..54425ad28bfd 100644
--- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
+++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
@@ -103,23 +103,27 @@ static void sdio_free_irq(struct dvobj_priv *dvobj)
struct sdio_func *func;
int err;
- if (dvobj->irq_alloc) {
- psdio_data = >intf_data;
- func = psdio_data->func;
-
- if (func) {
- sdio_claim_host(func);
- err = sdio_release_irq(func);
- if (err) {
- dvobj->drv_dbg.dbg_sdio_free_irq_error_cnt++;
- DBG_871X_LEVEL(_drv_err_, "%s: sdio_release_irq
FAIL(%d)!\n", __func__, err);
- } else {
- dvobj->drv_dbg.dbg_sdio_free_irq_cnt++;
- }
- sdio_release_host(func);
- }
- dvobj->irq_alloc = 0;
+ if (!dvobj->irq_alloc)
+ return;
+
+ psdio_data = >intf_data;
+ func = psdio_data->func;
+ dvobj->irq_alloc = 0;
+
+ if (!func)
+ return;
+
+ sdio_claim_host(func);
+ err = sdio_release_irq(func);
+ if (err) {
+ dvobj->drv_dbg.dbg_sdio_free_irq_error_cnt++;
+ DBG_871X_LEVEL(_drv_err_, "%s: sdio_release_irq FAIL(%d)!\n",
+ __func__, err);
+ } else {
+ dvobj->drv_dbg.dbg_sdio_free_irq_cnt++;
}
+
+ sdio_release_host(func);
}
#ifdef CONFIG_GPIO_WAKEUP
@@ -219,26 +223,29 @@ static void sdio_deinit(struct dvobj_priv *dvobj)
func = dvobj->intf_data.func;
- if (func) {
- sdio_claim_host(func);
- err = sdio_disable_func(func);
- if (err) {
- dvobj->drv_dbg.dbg_sdio_deinit_error_cnt++;
- DBG_8192C(KERN_ERR "%s: sdio_disable_func(%d)\n",
__func__, err);
- }
+ if (!func)
+ return;
- if (dvobj->irq_alloc) {
- err = sdio_release_irq(func);
- if (err) {
- dvobj->drv_dbg.dbg_sdio_free_irq_error_cnt++;
- DBG_8192C(KERN_ERR "%s:
sdio_release_irq(%d)\n", __func__, err);
- } else {
- dvobj->drv_dbg.dbg_sdio_free_irq_cnt++;
- }
- }
+ sdio_claim_host(func);
+ err = sdio_disable_func(func);
+ if (err) {
+ dvobj->drv_dbg.dbg_sdio_deinit_error_cnt++;
+ DBG_8192C(KERN_ERR "%s: sdio_disable_func(%d)\n",
+ __func__, err);
+ }
- sdio_release_host(func);
+ if (dvobj->irq_alloc) {
+ err = sdio_release_irq(func);
+ if (err) {
+ dvobj->drv_dbg.dbg_sdio_free_irq_error_cnt++;
+ DBG_8192C(KERN_ERR "%s: sdio_release_irq(%d)\n",
+ __func__, err);
+ } else {
+ dvobj->drv_dbg.dbg_sdio_free_irq_cnt++;
+ }
}
+
+ sdio_release_host(func);
}
static struct dvobj_priv *sdio_dvobj_init(struct sdio_func *func)
--
2.17.1
[PATCH 1/2] Staging: rtl8723bs: os_dep: Fix several coding style errors
From: Guilherme T Maeoka
Fix coding style errors and warns complained by checkpatck.pl. To list:
- remove braces for single statements blocks,
- add space required around operators,
- replace spaces with tabs to indent,
- add blank line after declarations,
- fix braces and 'else' poistions in selection statements.
Signed-off-by: Guilherme T Maeoka
---
drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 112 +--
1 file changed, 50 insertions(+), 62 deletions(-)
diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
index 6d02904de63f..86f1e090436e 100644
--- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
+++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
@@ -14,14 +14,14 @@
#define dev_to_sdio_func(d) container_of(d, struct sdio_func, dev)
#endif
-static const struct sdio_device_id sdio_ids[] =
-{
+static const struct sdio_device_id sdio_ids[] = {
{ SDIO_DEVICE(0x024c, 0x0523), },
{ SDIO_DEVICE(0x024c, 0x0623), },
{ SDIO_DEVICE(0x024c, 0x0626), },
{ SDIO_DEVICE(0x024c, 0xb723), },
{ /* end: all zeroes */ },
};
+
static const struct acpi_device_id acpi_ids[] = {
{"OBDA8723", 0x},
{}
@@ -84,46 +84,42 @@ static int sdio_alloc_irq(struct dvobj_priv *dvobj)
sdio_claim_host(func);
err = sdio_claim_irq(func, _sync_int_hdl);
- if (err)
- {
+ if (err) {
dvobj->drv_dbg.dbg_sdio_alloc_irq_error_cnt++;
printk(KERN_CRIT "%s: sdio_claim_irq FAIL(%d)!\n", __func__,
err);
- }
- else
- {
+ } else {
dvobj->drv_dbg.dbg_sdio_alloc_irq_cnt++;
dvobj->irq_alloc = 1;
}
sdio_release_host(func);
- return err?_FAIL:_SUCCESS;
+ return err ? _FAIL : _SUCCESS;
}
static void sdio_free_irq(struct dvobj_priv *dvobj)
{
-PSDIO_DATA psdio_data;
-struct sdio_func *func;
-int err;
-
-if (dvobj->irq_alloc) {
-psdio_data = >intf_data;
-func = psdio_data->func;
-
-if (func) {
-sdio_claim_host(func);
-err = sdio_release_irq(func);
-if (err)
-{
+ PSDIO_DATA psdio_data;
+ struct sdio_func *func;
+ int err;
+
+ if (dvobj->irq_alloc) {
+ psdio_data = >intf_data;
+ func = psdio_data->func;
+
+ if (func) {
+ sdio_claim_host(func);
+ err = sdio_release_irq(func);
+ if (err) {
dvobj->drv_dbg.dbg_sdio_free_irq_error_cnt++;
- DBG_871X_LEVEL(_drv_err_,"%s: sdio_release_irq
FAIL(%d)!\n", __func__, err);
-}
-else
- dvobj->drv_dbg.dbg_sdio_free_irq_cnt++;
-sdio_release_host(func);
-}
-dvobj->irq_alloc = 0;
-}
+ DBG_871X_LEVEL(_drv_err_, "%s: sdio_release_irq
FAIL(%d)!\n", __func__, err);
+ } else {
+ dvobj->drv_dbg.dbg_sdio_free_irq_cnt++;
+ }
+ sdio_release_host(func);
+ }
+ dvobj->irq_alloc = 0;
+ }
}
#ifdef CONFIG_GPIO_WAKEUP
@@ -131,16 +127,18 @@ extern unsigned int oob_irq;
static irqreturn_t gpio_hostwakeup_irq_thread(int irq, void *data)
{
struct adapter *padapter = data;
+
DBG_871X_LEVEL(_drv_always_, "gpio_hostwakeup_irq_thread\n");
/* Disable interrupt before calling handler */
/* disable_irq_nosync(oob_irq); */
- rtw_lock_suspend_timeout(HZ/2);
+ rtw_lock_suspend_timeout(HZ / 2);
return IRQ_HANDLED;
}
static u8 gpio_hostwakeup_alloc_irq(struct adapter *padapter)
{
int err;
+
if (oob_irq == 0) {
DBG_871X("oob_irq ZERO!\n");
return _FAIL;
@@ -150,9 +148,9 @@ static u8 gpio_hostwakeup_alloc_irq(struct adapter
*padapter)
/* and failing can prevent can not sleep issue if */
/* wifi gpio12 pin is not linked with CPU */
err = request_threaded_irq(oob_irq, gpio_hostwakeup_irq_thread, NULL,
- /* IRQF_TRIGGER_LOW | IRQF_ONESHOT, */
- IRQF_TRIGGER_FALLING,
- "rtw_wifi_gpio_wakeup", padapter);
+ /* IRQF_TRIGGER_LOW | IRQF_ONESHOT, */
+ IRQF_TRIGGER_FALLING,
+ "rtw_wifi_gpio_wakeup", padapter);
if (err < 0) {
DBG_871X("Oops: can't allocate gpio irq %d err:%d\n", oob_irq,
err);
return false;
@@ -224,26 +222,25 @@ static void sdio_deinit(struct dvobj_priv *dvobj)
if (func) {
sdio_claim_host(func);
err = sdio_disable_func(func);
- if (err)
Re: [RFC] simple_lmk: Introduce Simple Low Memory Killer for Android
On Tue, Mar 12, 2019 at 1:05 AM Michal Hocko wrote: > > On Mon 11-03-19 15:15:35, Suren Baghdasaryan wrote: > > On Mon, Mar 11, 2019 at 1:46 PM Sultan Alsawaf > > wrote: > > > > > > On Mon, Mar 11, 2019 at 01:10:36PM -0700, Suren Baghdasaryan wrote: > > > > The idea seems interesting although I need to think about this a bit > > > > more. Killing processes based on failed page allocation might backfire > > > > during transient spikes in memory usage. > > > > > > This issue could be alleviated if tasks could be killed and have their > > > pages > > > reaped faster. Currently, Linux takes a _very_ long time to free a task's > > > memory > > > after an initial privileged SIGKILL is sent to a task, even with the > > > task's > > > priority being set to the highest possible (so unwanted scheduler > > > preemption > > > starving dying tasks of CPU time is not the issue at play here). I've > > > frequently measured the difference in time between when a SIGKILL is sent > > > for a > > > task and when free_task() is called for that task to be hundreds of > > > milliseconds, which is incredibly long. AFAIK, this is a problem that LMKD > > > suffers from as well, and perhaps any OOM killer implementation in Linux, > > > since > > > you cannot evaluate effect you've had on memory pressure by killing a > > > process > > > for at least several tens of milliseconds. > > > > Yeah, killing speed is a well-known problem which we are considering > > in LMKD. For example the recent LMKD change to assign process being > > killed to a cpuset cgroup containing big cores cuts the kill time > > considerably. This is not ideal and we are thinking about better ways > > to expedite the cleanup process. > > If you design is relies on the speed of killing then it is fundamentally > flawed AFAICT. You cannot assume anything about how quickly a task dies. > It might be blocked in an uninterruptible sleep or performin an > operation which takes some time. Sure, oom_reaper might help here but > still. That's what I was considering. This is not a silver bullet but increased speed would not hurt. > The only way to control the OOM behavior pro-actively is to throttle > allocation speed. We have memcg high limit for that purpose. Along with > PSI, I can imagine a reasonably working user space early oom > notifications and reasonable acting upon that. That makes sense and we are working in this direction. > -- > Michal Hocko > SUSE Labs Thanks, Suren.
[PATCH] dmaengine: tegra210-adma: use devm_clk_*() helpers
Usage of pm_clk_*() results in non-zero prepare_count for clocks and hence
module clocks remain ON always. This is not desired as it will leak power
unncessarily. This patch replaces pm_clk_*() with devm_clk_*() interface.
This helps to keep refcounts balanced when device is not in use and runtime
PM callbacks help to enable or disable clocks. System suspend/resume calls
can use pm_runtime_force_suspend/resume.
Suggested-by: Mohan Kumar D
Reviewed-by: Jonathan Hunter
Signed-off-by: Sameer Pujar
---
drivers/dma/tegra210-adma.c | 37 ++---
1 file changed, 14 insertions(+), 23 deletions(-)
diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
index 5ec0dd9..be29171 100644
--- a/drivers/dma/tegra210-adma.c
+++ b/drivers/dma/tegra210-adma.c
@@ -22,7 +22,6 @@
#include
#include
#include
-#include
#include
#include
@@ -141,6 +140,7 @@ struct tegra_adma {
struct dma_device dma_dev;
struct device *dev;
void __iomem*base_addr;
+ struct clk *ahub_clk;
unsigned intnr_channels;
unsigned long rx_requests_reserved;
unsigned long tx_requests_reserved;
@@ -637,8 +637,9 @@ static int tegra_adma_runtime_suspend(struct device *dev)
struct tegra_adma *tdma = dev_get_drvdata(dev);
tdma->global_cmd = tdma_read(tdma, ADMA_GLOBAL_CMD);
+ clk_disable_unprepare(tdma->ahub_clk);
- return pm_clk_suspend(dev);
+ return 0;
}
static int tegra_adma_runtime_resume(struct device *dev)
@@ -646,10 +647,11 @@ static int tegra_adma_runtime_resume(struct device *dev)
struct tegra_adma *tdma = dev_get_drvdata(dev);
int ret;
- ret = pm_clk_resume(dev);
- if (ret)
+ ret = clk_prepare_enable(tdma->ahub_clk);
+ if (ret) {
+ dev_err(dev, "ahub clk_enable failed: %d\n", ret);
return ret;
-
+ }
tdma_write(tdma, ADMA_GLOBAL_CMD, tdma->global_cmd);
return 0;
@@ -693,13 +695,11 @@ static int tegra_adma_probe(struct platform_device *pdev)
if (IS_ERR(tdma->base_addr))
return PTR_ERR(tdma->base_addr);
- ret = pm_clk_create(>dev);
- if (ret)
- return ret;
-
- ret = of_pm_clk_add_clk(>dev, "d_audio");
- if (ret)
- goto clk_destroy;
+ tdma->ahub_clk = devm_clk_get(>dev, "d_audio");
+ if (IS_ERR(tdma->ahub_clk)) {
+ dev_err(>dev, "Error: Missing ahub controller clock\n");
+ return PTR_ERR(tdma->ahub_clk);
+ }
pm_runtime_enable(>dev);
@@ -776,8 +776,6 @@ static int tegra_adma_probe(struct platform_device *pdev)
pm_runtime_put_sync(>dev);
rpm_disable:
pm_runtime_disable(>dev);
-clk_destroy:
- pm_clk_destroy(>dev);
return ret;
}
@@ -794,22 +792,15 @@ static int tegra_adma_remove(struct platform_device *pdev)
pm_runtime_put_sync(>dev);
pm_runtime_disable(>dev);
- pm_clk_destroy(>dev);
return 0;
}
-#ifdef CONFIG_PM_SLEEP
-static int tegra_adma_pm_suspend(struct device *dev)
-{
- return pm_runtime_suspended(dev) == false;
-}
-#endif
-
static const struct dev_pm_ops tegra_adma_dev_pm_ops = {
SET_RUNTIME_PM_OPS(tegra_adma_runtime_suspend,
tegra_adma_runtime_resume, NULL)
- SET_SYSTEM_SLEEP_PM_OPS(tegra_adma_pm_suspend, NULL)
+ SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+ pm_runtime_force_resume)
};
static struct platform_driver tegra_admac_driver = {
--
2.7.4
[PATCH] bus: tegra-aconnect: use devm_clk_*() helpers
aconnect bus driver is using pm_clk_*() for managing required clocks. With
this, clocks seem to be always ON. This happens because, the clock prepare
count is incremented in pm_clock_acquire() which is called during device
probe(). The count is decremented during remove(). Hence the prepare_count
for the clock is non-zero till the remove() gets executed. This is true for
BPMP managed clock sources, where clock enable and disable happens during
prepare and unprepare phases respectively. Thus clocks remain ON always and
that is why pm_clk_*() cannot be used on Tegra.
This patch replaces pm_clk_*() with devm_clk_*() and runtime PM callbacks
reflect this. System suspend/resume can use pm_runtime_force_suspend/resume
Suggested-by: Mohan Kumar D
Reviewed-by: Jonathan Hunter
Signed-off-by: Sameer Pujar
---
drivers/bus/tegra-aconnect.c | 66 ++--
1 file changed, 46 insertions(+), 20 deletions(-)
diff --git a/drivers/bus/tegra-aconnect.c b/drivers/bus/tegra-aconnect.c
index 084ae28..ac58142 100644
--- a/drivers/bus/tegra-aconnect.c
+++ b/drivers/bus/tegra-aconnect.c
@@ -12,28 +12,38 @@
#include
#include
#include
-#include
#include
+struct tegra_aconnect {
+ struct clk *ape_clk;
+ struct clk *apb2ape_clk;
+};
+
static int tegra_aconnect_probe(struct platform_device *pdev)
{
- int ret;
+ struct tegra_aconnect *aconnect;
if (!pdev->dev.of_node)
return -EINVAL;
- ret = pm_clk_create(>dev);
- if (ret)
- return ret;
+ aconnect = devm_kzalloc(>dev, sizeof(struct tegra_aconnect),
+ GFP_KERNEL);
+ if (!aconnect)
+ return -ENOMEM;
- ret = of_pm_clk_add_clk(>dev, "ape");
- if (ret)
- goto clk_destroy;
+ aconnect->ape_clk = devm_clk_get(>dev, "ape");
+ if (IS_ERR(aconnect->ape_clk)) {
+ dev_err(>dev, "Can't retrieve ape clock\n");
+ return PTR_ERR(aconnect->ape_clk);
+ }
- ret = of_pm_clk_add_clk(>dev, "apb2ape");
- if (ret)
- goto clk_destroy;
+ aconnect->apb2ape_clk = devm_clk_get(>dev, "apb2ape");
+ if (IS_ERR(aconnect->apb2ape_clk)) {
+ dev_err(>dev, "Can't retrieve apb2ape clock\n");
+ return PTR_ERR(aconnect->apb2ape_clk);
+ }
+ dev_set_drvdata(>dev, aconnect);
pm_runtime_enable(>dev);
of_platform_populate(pdev->dev.of_node, NULL, NULL, >dev);
@@ -41,35 +51,51 @@ static int tegra_aconnect_probe(struct platform_device
*pdev)
dev_info(>dev, "Tegra ACONNECT bus registered\n");
return 0;
-
-clk_destroy:
- pm_clk_destroy(>dev);
-
- return ret;
}
static int tegra_aconnect_remove(struct platform_device *pdev)
{
pm_runtime_disable(>dev);
- pm_clk_destroy(>dev);
-
return 0;
}
static int tegra_aconnect_runtime_resume(struct device *dev)
{
- return pm_clk_resume(dev);
+ struct tegra_aconnect *aconnect = dev_get_drvdata(dev);
+ int ret;
+
+ ret = clk_prepare_enable(aconnect->ape_clk);
+ if (ret) {
+ dev_err(dev, "ape clk_enable failed: %d\n", ret);
+ return ret;
+ }
+
+ ret = clk_prepare_enable(aconnect->apb2ape_clk);
+ if (ret) {
+ clk_disable_unprepare(aconnect->ape_clk);
+ dev_err(dev, "apb2ape clk_enable failed: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
}
static int tegra_aconnect_runtime_suspend(struct device *dev)
{
- return pm_clk_suspend(dev);
+ struct tegra_aconnect *aconnect = dev_get_drvdata(dev);
+
+ clk_disable_unprepare(aconnect->ape_clk);
+ clk_disable_unprepare(aconnect->apb2ape_clk);
+
+ return 0;
}
static const struct dev_pm_ops tegra_aconnect_pm_ops = {
SET_RUNTIME_PM_OPS(tegra_aconnect_runtime_suspend,
tegra_aconnect_runtime_resume, NULL)
+ SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+ pm_runtime_force_resume)
};
static const struct of_device_id tegra_aconnect_of_match[] = {
--
2.7.4
Re: INFO: rcu detected stall in sys_sendfile64 (2)
(Moving most recipients to bcc: in order to avoid flooding.) On 2019/03/12 13:08, Al Viro wrote: > Umm... Might be a good idea to add some plausibility filters - it is, > in theory, possible that adding a line in a comment changes behaviour > (without compiler bugs, even - playing with __LINE__ is all it would > take), but the odds that it's _not_ a false positive are very low. Well, 108 out of 168 tests done during this bisection failed to test. With such high failure ratio, it is possible that by chance no crash happened during few tests for specific commit; causing a wrong bisection result. I expect that when trying to conclude "git bisect good" for specific commit, the tests should be repeated until no crash happened during 8 successful tests. Also, this bisection is finding multiple different crash patterns, which suggests that the crashed tests are not giving correct feedback to syzbot. $ grep -F 'run #' bisect.txt\?x\=1322028320 | wc -l 168 $ grep -F 'Connection timed out' bisect.txt\?x\=1322028320 | wc -l 108 $ grep -F 'crashed' bisect.txt\?x\=1322028320 run #0: crashed: WARNING: ODEBUG bug in netdev_freemem run #0: crashed: WARNING: ODEBUG bug in netdev_freemem run #1: crashed: INFO: rcu detected stall in corrupted run #0: crashed: INFO: rcu detected stall in sys_sendfile64 run #0: crashed: INFO: rcu detected stall in corrupted run #4: crashed: INFO: rcu detected stall in sys_sendfile64 run #0: crashed: INFO: rcu detected stall in corrupted run #1: crashed: INFO: rcu detected stall in corrupted run #0: crashed: INFO: rcu detected stall in ext4_file_write_iter run #0: crashed: INFO: rcu detected stall in corrupted run #0: crashed: INFO: rcu detected stall in sendfile64 run #0: crashed: INFO: rcu detected stall in corrupted run #1: crashed: INFO: rcu detected stall in sendfile64 run #0: crashed: INFO: rcu detected stall in ext4_file_write_iter run #1: crashed: INFO: rcu detected stall in corrupted run #0: crashed: INFO: rcu detected stall in corrupted run #0: crashed: INFO: rcu detected stall in corrupted run #0: crashed: INFO: rcu detected stall in corrupted run #1: crashed: INFO: rcu detected stall in corrupted run #0: crashed: INFO: rcu detected stall in corrupted run #3: crashed: INFO: rcu detected stall in corrupted run #0: crashed: INFO: rcu detected stall in do_iter_write
[PATCH] arm64: tegra: dts: enable aconnect, adma and agic
Enable aconnect, adma and agic for Tegra Jetson TX1.
Signed-off-by: Sameer Pujar
---
arch/arm64/boot/dts/nvidia/tegra210-p2371-2180.dts | 12
1 file changed, 12 insertions(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra210-p2371-2180.dts
b/arch/arm64/boot/dts/nvidia/tegra210-p2371-2180.dts
index 9fad0d2..5a57396 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210-p2371-2180.dts
+++ b/arch/arm64/boot/dts/nvidia/tegra210-p2371-2180.dts
@@ -99,4 +99,16 @@
pinctrl-0 = <_pwm_active_state>;
pinctrl-1 = <_pwm_inactive_state>;
};
+
+ aconnect@702c {
+ status = "okay";
+
+ dma@702e2000 {
+ status = "okay";
+ };
+
+ agic@702f9000 {
+ status = "okay";
+ };
+ };
};
--
2.7.4
Re: [PATCH v6 1/2] dt-bindings: misc: aspeed-p2a-ctrl: add support
On Mon, Mar 11, 2019 at 7:38 PM Rob Herring wrote:
>
> On Mon, Mar 11, 2019 at 6:49 PM Patrick Venture wrote:
> >
> > On Mon, Mar 11, 2019 at 3:20 PM Rob Herring wrote:
> > >
> > > On Mon, Mar 04, 2019 at 10:55:36AM -0800, Patrick Venture wrote:
> > > > Document the ast2400, ast2500 PCI-to-AHB bridge control driver bindings.
> > > >
> > > > Signed-off-by: Patrick Venture
> > > > ---
> > > > Changes for v6:
> > > > - None
> > > > Changes for v5:
> > > > - None
> > > > Changes for v4:
> > > > - None
> > > > Changes for v3:
> > > > - None
> > > > Changes for v2:
> > > > - Added comment about syscon required parameter.
> > > > ---
> > > > .../bindings/misc/aspeed-p2a-ctrl.txt | 32 +++
> > > > 1 file changed, 32 insertions(+)
> > > > create mode 100644
> > > > Documentation/devicetree/bindings/misc/aspeed-p2a-ctrl.txt
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/misc/aspeed-p2a-ctrl.txt
> > > > b/Documentation/devicetree/bindings/misc/aspeed-p2a-ctrl.txt
> > > > new file mode 100644
> > > > index ..1092d62d1c92
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/misc/aspeed-p2a-ctrl.txt
> > > > @@ -0,0 +1,32 @@
> > > > +==
> > > > +Device tree bindings for Aspeed AST2400/AST2500 PCI-to-AHB Bridge
> > > > Control Driver
> > > > +==
> > > > +
> > > > +The bridge is available on platforms with the VGA enabled on the
> > > > Aspeed device.
> > > > +In this case, the host has access to a 64KiB window into all of the
> > > > BMC's
> > > > +memory. The BMC can disable this bridge. If the bridge is enabled,
> > > > the host
> > > > +has read access to all the regions of memory, however the host only
> > > > has read
> > > > +and write access depending on a register controlled by the BMC.
> > > > +
> > > > +Required properties:
> > > > +===
> > > > +
> > > > + - compatible: must be one of:
> > > > + - "aspeed,ast2400-p2a-ctrl"
> > > > + - "aspeed,ast2500-p2a-ctrl"
> > > > +
> > > > + - syscon: handle to syscon device node controlling PCI.
> > > > +
> > > > +Optional properties:
> > > > +===
> > > > +
> > > > +- memory-region: A phandle to a reserved_memory region to be used for
> > > > the PCI
> > > > + to AHB mapping
> > > > +
> > > > +Example:
> > > > +
> > > > +p2a: p2a-control@1e6e2000 {
> > > > + compatible = "aspeed,ast2400-p2a-ctrl";
> > > > + memory-region = <_memory>;
> > > > + syscon = <>;
> > >
> > > Make this node a child of what you are pointing to instead if this the
> > > only control interface.
> >
> > You're suggesting I make this a child of the syscon?
>
> Yes.
Roger that, will update and send out a patchset v6 with the
corresponding changes.
>
> Rob
Re: [PATCH 2/2] xfs: clean up xfs_dir2_leaf_addname
On Mon, Mar 11, 2019 at 09:22:32AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong
>
> Remove typedefs and consolidate local variable initialization.
>
> Signed-off-by: Darrick J. Wong
Reviewed-by: Bill O'Donnell
> ---
> fs/xfs/libxfs/xfs_dir2_leaf.c | 33 +++--
> 1 file changed, 15 insertions(+), 18 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
> index 2abf945e5844..9c2a0a13ed61 100644
> --- a/fs/xfs/libxfs/xfs_dir2_leaf.c
> +++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
> @@ -563,43 +563,40 @@ xfs_dir3_leaf_find_entry(
> */
> int /* error */
> xfs_dir2_leaf_addname(
> - xfs_da_args_t *args) /* operation arguments */
> + struct xfs_da_args *args) /* operation arguments */
> {
> + struct xfs_dir3_icleaf_hdr leafhdr;
> + struct xfs_trans*tp = args->trans;
> __be16 *bestsp;/* freespace table in leaf */
> - int compact;/* need to compact leaves */
> - xfs_dir2_data_hdr_t *hdr; /* data block header */
> + __be16 *tagp; /* end of data entry */
> struct xfs_buf *dbp; /* data block buffer */
> - xfs_dir2_data_entry_t *dep; /* data block entry */
> - xfs_inode_t *dp;/* incore directory inode */
> - xfs_dir2_data_unused_t *dup; /* data unused entry */
> + struct xfs_buf *lbp; /* leaf's buffer */
> + struct xfs_dir2_leaf*leaf; /* leaf structure */
> + struct xfs_inode*dp = args->dp; /* incore directory inode */
> + struct xfs_dir2_data_hdr *hdr; /* data block header */
> + struct xfs_dir2_data_entry *dep;/* data block entry */
> + struct xfs_dir2_leaf_entry *lep;/* leaf entry table pointer */
> + struct xfs_dir2_leaf_entry *ents;
> + struct xfs_dir2_data_unused *dup; /* data unused entry */
> + struct xfs_dir2_leaf_tail *ltp; /* leaf tail pointer */
> + struct xfs_dir2_data_free *bf; /* bestfree table */
> + int compact;/* need to compact leaves */
> int error; /* error return value */
> int grown; /* allocated new data block */
> int highstale = 0; /* index of next stale leaf */
> int i; /* temporary, index */
> int index; /* leaf table position */
> - struct xfs_buf *lbp; /* leaf's buffer */
> - xfs_dir2_leaf_t *leaf; /* leaf structure */
> int length; /* length of new entry */
> - xfs_dir2_leaf_entry_t *lep; /* leaf entry table pointer */
> int lfloglow; /* low leaf logging index */
> int lfloghigh; /* high leaf logging index */
> int lowstale = 0; /* index of prev stale leaf */
> - xfs_dir2_leaf_tail_t*ltp; /* leaf tail pointer */
> int needbytes; /* leaf block bytes needed */
> int needlog;/* need to log data header */
> int needscan; /* need to rescan data free */
> - __be16 *tagp; /* end of data entry */
> - xfs_trans_t *tp;/* transaction pointer */
> xfs_dir2_db_t use_block; /* data block number */
> - struct xfs_dir2_data_free *bf; /* bestfree table */
> - struct xfs_dir2_leaf_entry *ents;
> - struct xfs_dir3_icleaf_hdr leafhdr;
>
> trace_xfs_dir2_leaf_addname(args);
>
> - dp = args->dp;
> - tp = args->trans;
> -
> error = xfs_dir3_leaf_read(tp, dp, args->geo->leafblk, -1, );
> if (error)
> return error;
Re: [PATCH 1/2] xfs: zero initialize highstale and lowstale in xfs_dir2_leaf_addname
On Mon, Mar 11, 2019 at 09:19:48AM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong > > Smatch complains about the following: > > fs/xfs/libxfs/xfs_dir2_leaf.c:848 xfs_dir2_leaf_addname() error: > uninitialized symbol 'lowstale'. > > fs/xfs/libxfs/xfs_dir2_leaf.c:849 xfs_dir2_leaf_addname() error: > uninitialized symbol 'highstale'. > > I don't think there's any incorrect behavior associated with the > uninitialized variable, but as the author of the previous zero-init > patch points out, it's best not to be passing around pointers to > uninitialized stack areas. > > Signed-off-by: Darrick J. Wong Reviewed-by: Bill O'Donnell > --- > fs/xfs/libxfs/xfs_dir2_leaf.c |4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c > index 9a3767818c50..2abf945e5844 100644 > --- a/fs/xfs/libxfs/xfs_dir2_leaf.c > +++ b/fs/xfs/libxfs/xfs_dir2_leaf.c > @@ -574,7 +574,7 @@ xfs_dir2_leaf_addname( > xfs_dir2_data_unused_t *dup; /* data unused entry */ > int error; /* error return value */ > int grown; /* allocated new data block */ > - int highstale; /* index of next stale leaf */ > + int highstale = 0; /* index of next stale leaf */ > int i; /* temporary, index */ > int index; /* leaf table position */ > struct xfs_buf *lbp; /* leaf's buffer */ > @@ -583,7 +583,7 @@ xfs_dir2_leaf_addname( > xfs_dir2_leaf_entry_t *lep; /* leaf entry table pointer */ > int lfloglow; /* low leaf logging index */ > int lfloghigh; /* high leaf logging index */ > - int lowstale; /* index of prev stale leaf */ > + int lowstale = 0; /* index of prev stale leaf */ > xfs_dir2_leaf_tail_t*ltp; /* leaf tail pointer */ > int needbytes; /* leaf block bytes needed */ > int needlog;/* need to log data header */
[PATCH] arm64: defconfig: build aconnect and adma drivers as modules
Following configs/drivers are enabled as modules, - TEGRA_ACONNECT (drivers/bus/tegra-aconnect.c) - TEGRA210_ADMA (drivers/dma/tegra210-adma.c) Signed-off-by: Sameer Pujar --- arch/arm64/configs/defconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index 2d9c390..e9d514c 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -203,6 +203,7 @@ CONFIG_DMA_CMA=y CONFIG_CMA_SIZE_MBYTES=32 CONFIG_HISILICON_LPC=y CONFIG_SIMPLE_PM_BUS=y +CONFIG_TEGRA_ACONNECT=m CONFIG_MTD=y CONFIG_MTD_BLOCK=y CONFIG_MTD_M25P80=y @@ -622,6 +623,7 @@ CONFIG_K3_DMA=y CONFIG_MV_XOR_V2=y CONFIG_PL330_DMA=y CONFIG_TEGRA20_APB_DMA=y +CONFIG_TEGRA210_ADMA=m CONFIG_QCOM_BAM_DMA=y CONFIG_QCOM_HIDMA_MGMT=y CONFIG_QCOM_HIDMA=y -- 2.7.4
[PATCH v2 2/2] ceph: quota: fix quota subdir mounts
The CephFS kernel client does not enforce quotas set in a directory that
isn't visible from the mount point. For example, given the path
'/dir1/dir2', if quotas are set in 'dir1' and the filesystem is mounted with
mount -t ceph ::/dir1/ /mnt
then the client won't be able to access 'dir1' inode, even if 'dir2' belongs
to a quota realm that points to it.
This patch fixes this issue by simply doing an MDS LOOKUPINO operation for
unknown inodes. Any inode reference obtained this way will be added to a
list in ceph_mds_client, and will only be released when the filesystem is
umounted.
Link: https://tracker.ceph.com/issues/38482
Reported-by: Hendrik Peyerl
Signed-off-by: Luis Henriques
---
fs/ceph/mds_client.c | 15 ++
fs/ceph/mds_client.h | 2 +
fs/ceph/quota.c | 106 +++
fs/ceph/super.h | 2 +
4 files changed, 115 insertions(+), 10 deletions(-)
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 163fc74bf221..1dc24c3525fe 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -3656,6 +3656,8 @@ int ceph_mdsc_init(struct ceph_fs_client *fsc)
mdsc->max_sessions = 0;
mdsc->stopping = 0;
atomic64_set(>quotarealms_count, 0);
+ INIT_LIST_HEAD(>quotarealms_inodes_list);
+ spin_lock_init(>quotarealms_inodes_lock);
mdsc->last_snap_seq = 0;
init_rwsem(>snap_rwsem);
mdsc->snap_realms = RB_ROOT;
@@ -3726,6 +3728,8 @@ static void wait_requests(struct ceph_mds_client *mdsc)
*/
void ceph_mdsc_pre_umount(struct ceph_mds_client *mdsc)
{
+ struct ceph_inode_info *ci;
+
dout("pre_umount\n");
mdsc->stopping = 1;
@@ -3738,6 +3742,17 @@ void ceph_mdsc_pre_umount(struct ceph_mds_client *mdsc)
* their inode/dcache refs
*/
ceph_msgr_flush();
+ /*
+* It's now safe to clean quotarealms_inode_list without holding
+* mdsc->quotarealms_inodes_lock
+*/
+ while (!list_empty(>quotarealms_inodes_list)) {
+ ci = list_first_entry(>quotarealms_inodes_list,
+ struct ceph_inode_info,
+ i_quotarealms_inode_item);
+ list_del(>i_quotarealms_inode_item);
+ iput(>vfs_inode);
+ }
}
/*
diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
index 729da155ebf0..58968fb338ec 100644
--- a/fs/ceph/mds_client.h
+++ b/fs/ceph/mds_client.h
@@ -329,6 +329,8 @@ struct ceph_mds_client {
int stopping; /* true if shutting down */
atomic64_t quotarealms_count; /* # realms with quota */
+ struct list_headquotarealms_inodes_list;
+ spinlock_t quotarealms_inodes_lock;
/*
* snap_rwsem will cover cap linkage into snaprealms, and
diff --git a/fs/ceph/quota.c b/fs/ceph/quota.c
index 9455d3aef0c3..d1ab1b331c0d 100644
--- a/fs/ceph/quota.c
+++ b/fs/ceph/quota.c
@@ -22,7 +22,16 @@ void ceph_adjust_quota_realms_count(struct inode *inode,
bool inc)
static inline bool ceph_has_realms_with_quotas(struct inode *inode)
{
struct ceph_mds_client *mdsc = ceph_inode_to_client(inode)->mdsc;
- return atomic64_read(>quotarealms_count) > 0;
+ struct super_block *sb = mdsc->fsc->sb;
+
+ if (atomic64_read(>quotarealms_count) > 0)
+ return true;
+ /* if root is the real CephFS root, we don't have quota realms */
+ if (sb->s_root->d_inode &&
+ (sb->s_root->d_inode->i_ino == CEPH_INO_ROOT))
+ return false;
+ /* otherwise, we can't know for sure */
+ return true;
}
void ceph_handle_quota(struct ceph_mds_client *mdsc,
@@ -68,6 +77,37 @@ void ceph_handle_quota(struct ceph_mds_client *mdsc,
iput(inode);
}
+/*
+ * This function will try to lookup a realm inode. If the inode is found
+ * (through an MDS LOOKUPINO operation), the realm->inode will be updated and
+ * the inode will also be added to an mdsc list which will be freed only when
+ * the filesystem is umounted.
+ */
+static struct inode *lookup_quotarealm_inode(struct ceph_mds_client *mdsc,
+struct super_block *sb,
+struct ceph_snap_realm *realm)
+{
+ struct inode *in;
+
+ in = ceph_lookup_inode(sb, realm->ino);
+ if (IS_ERR(in)) {
+ pr_warn("Can't lookup inode %llx (err: %ld)\n",
+ realm->ino, PTR_ERR(in));
+ return in;
+ }
+
+ spin_lock(>quotarealms_inodes_lock);
+ list_add(_inode(in)->i_quotarealms_inode_item,
+>quotarealms_inodes_list);
+ spin_unlock(>quotarealms_inodes_lock);
+
+ spin_lock(>inodes_with_caps_lock);
+ realm->inode = in;
+ spin_unlock(>inodes_with_caps_lock);
+
+ return in;
+}
+
/*
* This function walks through the snaprealm for an
[PATCH v2 0/2] fix quota subdir mounts
Hi, As recently reported in the ceph-users mailing-list[1], the kernel client behaves differently from the fuse client regarding mounting subdirs where quotas are in effect. I've also created a bug to track this issue[2]. The following patches are a possible way of fixing this issue. The performance impact should be close to zero if the mount is done in the CephFS root inode. When we're mounting subdirs, we may have extra queries to the MDSs, depending on how many extra realms we'll need to loop through. Changes since v1: - Loop to free mdsc->quotarealms_inodes_list list was moved further down where it's not possible to race with insertions. This way there's no need to hold the spinlock anymore. - Clarified comments regarding the get_quota_realm function 'retry' parameter, both in the function itself and in function ceph_quota_is_same_realm, where that param is set to 'false' - Distinguish between 'realm->inode is NULL' and igrab failures, both in get_quota_realm and check_quota_exceeded Changes since RFC: The 1st patch hasn't been changed since the initial RFC. The 2nd patch has been refactored to include the following changes: - Zheng Yan's suggestions, i.e, move inode references from the realms to ceph_mds_client instance - It now also handles other cases where an MDS lookup may need to be performed: * statfs when there are quotas * renames, to forbid cross-quota renames [1] http://lists.ceph.com/pipermail/ceph-users-ceph.com/2019-February/033357.html [2] https://tracker.ceph.com/issues/38482 Cheers, -- Luís Luis Henriques (2): ceph: factor out ceph_lookup_inode() ceph: quota: fix quota subdir mounts fs/ceph/export.c | 14 +- fs/ceph/mds_client.c | 15 ++ fs/ceph/mds_client.h | 2 + fs/ceph/quota.c | 106 +++ fs/ceph/super.h | 3 ++ 5 files changed, 129 insertions(+), 11 deletions(-)
[PATCH v2 1/2] ceph: factor out ceph_lookup_inode()
This function will be used by __fh_to_dentry and by the quotas code, to
find quota realm inodes that are not visible in the mountpoint.
Signed-off-by: Luis Henriques
---
fs/ceph/export.c | 14 +-
fs/ceph/super.h | 1 +
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/fs/ceph/export.c b/fs/ceph/export.c
index 3c59ad180ef0..0d8ead82c816 100644
--- a/fs/ceph/export.c
+++ b/fs/ceph/export.c
@@ -59,7 +59,7 @@ static int ceph_encode_fh(struct inode *inode, u32 *rawfh,
int *max_len,
return type;
}
-static struct dentry *__fh_to_dentry(struct super_block *sb, u64 ino)
+struct inode *ceph_lookup_inode(struct super_block *sb, u64 ino)
{
struct ceph_mds_client *mdsc = ceph_sb_to_client(sb)->mdsc;
struct inode *inode;
@@ -92,12 +92,24 @@ static struct dentry *__fh_to_dentry(struct super_block
*sb, u64 ino)
ceph_mdsc_put_request(req);
if (!inode)
return ERR_PTR(-ESTALE);
+ if (err)
+ return ERR_PTR(err);
if (inode->i_nlink == 0) {
iput(inode);
return ERR_PTR(-ESTALE);
}
}
+ return inode;
+}
+
+static struct dentry *__fh_to_dentry(struct super_block *sb, u64 ino)
+{
+ struct inode *inode = ceph_lookup_inode(sb, ino);
+
+ if (IS_ERR(inode))
+ return ERR_CAST(inode);
+
return d_obtain_alias(inode);
}
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index dfb64a5211b6..ce51e98b08ec 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -1061,6 +1061,7 @@ extern long ceph_ioctl(struct file *file, unsigned int
cmd, unsigned long arg);
/* export.c */
extern const struct export_operations ceph_export_ops;
+struct inode *ceph_lookup_inode(struct super_block *sb, u64 ino);
/* locks.c */
extern __init void ceph_flock_init(void);
[PATCH v2 0/2] prevent mincore() page cache leaks
Here's a new version of the mincore() patches, with feedback from Andrew Morton applied. The IOCB_NOWAIT patch was dropped since David Chinner pointed out it's incomplete. We definitely want the first patch, while for the second Linus said: I think that's fine, and probably the right thing to do, but I also suspect that nobody actually cares ;( Whether or not somebody cares, we should hear of no breakage. If somebody does care after all, without second patch we might hear of breakage, so I would suggest applying it. It's not that complicated after all (famous last words?) Jiri Kosina (1): mm/mincore: make mincore() more conservative Vlastimil Babka (1): mm/mincore: provide mapped status when cached status is not allowed mm/mincore.c | 80 1 file changed, 68 insertions(+), 12 deletions(-) -- 2.20.1
[PATCH v2 1/2] mm/mincore: make mincore() more conservative
From: Jiri Kosina
The semantics of what mincore() considers to be resident is not completely
clear, but Linux has always (since 2.3.52, which is when mincore() was
initially done) treated it as "page is available in page cache".
That's potentially a problem, as that [in]directly exposes meta-information
about pagecache / memory mapping state even about memory not strictly belonging
to the process executing the syscall, opening possibilities for sidechannel
attacks.
Change the semantics of mincore() so that it only reveals pagecache information
for non-anonymous mappings that belog to files that the calling process could
(if it tried to) successfully open for writing; otherwise we'd be including
shared non-exclusive mappings, which
- is the sidechannel
- is not the usecase for mincore(), as that's primarily used for data, not
(shared) text
[mho...@suse.com: restructure can_do_mincore() conditions]
Originally-by: Linus Torvalds
Originally-by: Dominique Martinet
Cc: Dominique Martinet
Cc: Andy Lutomirski
Cc: Dave Chinner
Cc: Kevin Easton
Cc: Matthew Wilcox
Cc: Cyril Hrubis
Cc: Tejun Heo
Cc: Kirill A. Shutemov
Cc: Daniel Gruss
Signed-off-by: Jiri Kosina
Signed-off-by: Vlastimil Babka
Acked-by: Josh Snyder
Acked-by: Michal Hocko
Signed-off-by: Jiri Kosina
Signed-off-by: Vlastimil Babka
---
mm/mincore.c | 23 ++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/mm/mincore.c b/mm/mincore.c
index 218099b5ed31..c3f058bd0faf 100644
--- a/mm/mincore.c
+++ b/mm/mincore.c
@@ -169,6 +169,22 @@ static int mincore_pte_range(pmd_t *pmd, unsigned long
addr, unsigned long end,
return 0;
}
+static inline bool can_do_mincore(struct vm_area_struct *vma)
+{
+ if (vma_is_anonymous(vma))
+ return true;
+ if (!vma->vm_file)
+ return false;
+ /*
+* Reveal pagecache information only for non-anonymous mappings that
+* correspond to the files the calling process could (if tried) open
+* for writing; otherwise we'd be including shared non-exclusive
+* mappings, which opens a side channel.
+*/
+ return inode_owner_or_capable(file_inode(vma->vm_file)) ||
+ inode_permission(file_inode(vma->vm_file), MAY_WRITE) == 0;
+}
+
/*
* Do a chunk of "sys_mincore()". We've already checked
* all the arguments, we hold the mmap semaphore: we should
@@ -189,8 +205,13 @@ static long do_mincore(unsigned long addr, unsigned long
pages, unsigned char *v
vma = find_vma(current->mm, addr);
if (!vma || addr < vma->vm_start)
return -ENOMEM;
- mincore_walk.mm = vma->vm_mm;
end = min(vma->vm_end, addr + (pages << PAGE_SHIFT));
+ if (!can_do_mincore(vma)) {
+ unsigned long pages = DIV_ROUND_UP(end - addr, PAGE_SIZE);
+ memset(vec, 1, pages);
+ return pages;
+ }
+ mincore_walk.mm = vma->vm_mm;
err = walk_page_range(addr, end, _walk);
if (err < 0)
return err;
--
2.20.1
[PATCH v2 2/2] mm/mincore: provide mapped status when cached status is not allowed
After "mm/mincore: make mincore() more conservative" we sometimes restrict the
information about page cache residency, which needs to be done without breaking
existing userspace, as much as possible. Instead of returning with error, we
thus fake the results. For that we return residency values as 1, which should
be safer than faking them as 0, as there might theoretically exist code that
would try to fault in the page(s) in a loop until mincore() returns 1 for them.
Faking 1 however means that such code would not fault in a page even if it was
not truly in page cache, with possibly unwanted performance implications. We
can improve the situation by revisting the approach of 574823bfab82 ("Change
mincore() to count "mapped" pages rather than "cached" pages"), later reverted
by 30bac164aca7 and replaced by restricting/faking the results. In this patch
we apply the approach only to cases where page cache residency check is
restricted. Thus mincore() will return 0 for an unmapped page (which may or may
not be resident in a pagecache), and 1 after the process faults it in.
One potential downside is that mincore() users will be now able to recognize
when a previously mapped page was reclaimed. While that might be useful for
some attack scenarios, it is not as crucial as recognizing that somebody else
faulted the page in, which is the main reason we are making mincore() more
conservative. For detecting that pages being reclaimed, there are also other
existing ways anyway.
Cc: Jiri Kosina
Cc: Dominique Martinet
Cc: Andy Lutomirski
Cc: Dave Chinner
Cc: Kevin Easton
Cc: Matthew Wilcox
Cc: Cyril Hrubis
Cc: Tejun Heo
Cc: Kirill A. Shutemov
Cc: Daniel Gruss
Signed-off-by: Vlastimil Babka
---
mm/mincore.c | 67 +++-
1 file changed, 51 insertions(+), 16 deletions(-)
diff --git a/mm/mincore.c b/mm/mincore.c
index c3f058bd0faf..c9a265abc631 100644
--- a/mm/mincore.c
+++ b/mm/mincore.c
@@ -21,12 +21,23 @@
#include
#include
+/*
+ * mincore() page walk's private structure. Contains pointer to the array
+ * of return values to be set, and whether the current vma passed the
+ * can_do_mincore() check.
+ */
+struct mincore_walk_private {
+ unsigned char *vec;
+ bool can_check_pagecache;
+};
+
static int mincore_hugetlb(pte_t *pte, unsigned long hmask, unsigned long addr,
unsigned long end, struct mm_walk *walk)
{
#ifdef CONFIG_HUGETLB_PAGE
unsigned char present;
- unsigned char *vec = walk->private;
+ struct mincore_walk_private *walk_private = walk->private;
+ unsigned char *vec = walk_private->vec;
/*
* Hugepages under user process are always in RAM and never
@@ -35,7 +46,7 @@ static int mincore_hugetlb(pte_t *pte, unsigned long hmask,
unsigned long addr,
present = pte && !huge_pte_none(huge_ptep_get(pte));
for (; addr != end; vec++, addr += PAGE_SIZE)
*vec = present;
- walk->private = vec;
+ walk_private->vec = vec;
#else
BUG();
#endif
@@ -85,7 +96,8 @@ static unsigned char mincore_page(struct address_space
*mapping, pgoff_t pgoff)
}
static int __mincore_unmapped_range(unsigned long addr, unsigned long end,
- struct vm_area_struct *vma, unsigned char *vec)
+ struct vm_area_struct *vma, unsigned char *vec,
+ bool can_check_pagecache)
{
unsigned long nr = (end - addr) >> PAGE_SHIFT;
int i;
@@ -95,7 +107,14 @@ static int __mincore_unmapped_range(unsigned long addr,
unsigned long end,
pgoff = linear_page_index(vma, addr);
for (i = 0; i < nr; i++, pgoff++)
- vec[i] = mincore_page(vma->vm_file->f_mapping, pgoff);
+ /*
+* Return page cache residency state if we are allowed
+* to, otherwise return mapping state, which is 0 for
+* an unmapped range.
+*/
+ vec[i] = can_check_pagecache ?
+mincore_page(vma->vm_file->f_mapping, pgoff)
+: 0;
} else {
for (i = 0; i < nr; i++)
vec[i] = 0;
@@ -106,8 +125,11 @@ static int __mincore_unmapped_range(unsigned long addr,
unsigned long end,
static int mincore_unmapped_range(unsigned long addr, unsigned long end,
struct mm_walk *walk)
{
- walk->private += __mincore_unmapped_range(addr, end,
- walk->vma, walk->private);
+ struct mincore_walk_private *walk_private = walk->private;
+ unsigned char *vec = walk_private->vec;
+
+ walk_private->vec += __mincore_unmapped_range(addr, end, walk->vma,
+ vec, walk_private->can_check_pagecache);
Re: [PATCH] mm: vmscan: show zone type in kswapd tracepoints
On Tue, Mar 12, 2019 at 9:38 PM Michal Hocko wrote: > > On Tue 12-03-19 19:04:43, Yafang Shao wrote: > > On Mon, Mar 11, 2019 at 4:47 PM Michal Hocko wrote: > > > > > > On Fri 01-03-19 15:38:54, Yafang Shao wrote: > > > > If we want to know the zone type, we have to check whether > > > > CONFIG_ZONE_DMA, CONFIG_ZONE_DMA32 and CONFIG_HIGHMEM are set or not, > > > > that's not so convenient. > > > > > > > > We'd better show the zone type directly. > > > > > > I do agree that zone number is quite PITA to process in general but do > > > we really need this information in the first place? Why do we even care? > > > > > > > Sometimes we want to know this event occurs in which zone, then we can > > get the information of this zone, > > for example via /proc/zoneinfo. > > It could give us more information for debugging. > > Could you be more specific please? > Honestly speaking, this one hasn't help us fix the real issue yet. > > > Zones are an MM internal implementation details and the more we export > > > to the userspace the more we are going to argue about breaking userspace > > > when touching them. So I would rather not export that information unless > > > it is terribly useful. > > > > > > > I 'm not sure whether zone type is terribly useful or not, but the > > 'zid' is useless at all. > > > > I don't agree that Zones are MM internal. > > We can get the zone type in many ways, for example /proc/zoneinfo. > > > > If we show this event occurs in which zone, we'd better show the zone type, > > or we should drop this 'zid'. > > Yes, I am suggesting the later. If somebody really needs it then I would > like to see a _specific_ usecase. Then we can add the proper name. This 'zid' always seems like a noise currently. I will send a patch to drop this one. Thanks Yafang
Re: [PATCH] rcu/tree: Fix self wakeups for grace period kthread
On Tue, Mar 12, 2019 at 05:25:28PM +0530, Neeraj Upadhyay wrote:
> On 3/12/19 7:20 AM, Steven Rostedt wrote:
> >On Fri, 8 Mar 2019 15:16:18 +0530
> >Neeraj Upadhyay wrote:
> >
> >>Update the code to match the comment that self wakeup of
> >>grace period kthread is allowed from interrupt handler, and
> >>softirq handler, running in the grace period kthread's
> >>context. Present code allows self wakeups from all
> >>interrupt contexts - nmi, softirq and hardirq contexts.
> >
> >That's not actually the issue. But it appears that we return if we
> >simply have BH disabled, which I don't think we want, and we don't care
> >about NMI as NMI should never call this code.
> >
> >I think your patch is correct, but the change log is not.
How about this?
The current rcu_gp_kthread_wake() function uses in_interrupt()
and thus does a self-wakeup from all interrupt contexts,
including the pointless case where the GP kthread happens to be
running with bottom halves disabled, along with the impossible
case where the GP kthread is running within an NMI handler (you
are not supposed to invoke rcu_gp_kthread_wake() from within an
NMI handler. This commit therefore replaces the in_interrupt()
with in_irq(), so that the self-wakeups happen only from handlers
for hardware interrupts and softirqs. This also makes the code
match the comment.
Thanx, Paul
> >-- Steve
> >
>
> Hi Steve, sorry, I don't understand fully, why we want to not return
> in BH disabled case. From the commit logs and lkml discussion, there
> is a case where GP kthread is interrupted in the wait event path and
> rcu_gp_kthread_wake() is called in softirq handler (I am not sure
> about interrupt handler case; how rcu_gp_kthread_wake() is called
> from that path).
>
> https://github.com/torvalds/linux/commit/1d1f898df6586c5ea9aeaf349f13089c6fa37903
>
> Thanks
> Neeraj
> >
> >>
> >>Signed-off-by: Neeraj Upadhyay
> >>---
> >> kernel/rcu/tree.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >>diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> >>index acd6ccf..57cac6d 100644
> >>--- a/kernel/rcu/tree.c
> >>+++ b/kernel/rcu/tree.c
> >>@@ -1585,7 +1585,7 @@ static bool rcu_future_gp_cleanup(struct rcu_node
> >>*rnp)
> >> static void rcu_gp_kthread_wake(void)
> >> {
> >>if ((current == rcu_state.gp_kthread &&
> >>-!in_interrupt() && !in_serving_softirq()) ||
> >>+!in_irq() && !in_serving_softirq()) ||
> >>!READ_ONCE(rcu_state.gp_flags) ||
> >>!rcu_state.gp_kthread)
> >>return;
> >
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
> member of the Code Aurora Forum, hosted by The Linux Foundation
>
Re: [PATCH] mm: remove unused variable
wt., 12 mar 2019 o 14:59 Khalid Aziz napisał(a):
>
> On 3/12/19 7:28 AM, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski
> >
> > The mm variable is set but unused. Remove it.
>
> It is used. Look further down for calls to set_pte_at().
>
> --
> Khalid
>
> >
> > Signed-off-by: Bartosz Golaszewski
> > ---
> > mm/mprotect.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/mm/mprotect.c b/mm/mprotect.c
> > index 028c724dcb1a..130dac3ad04f 100644
> > --- a/mm/mprotect.c
> > +++ b/mm/mprotect.c
> > @@ -39,7 +39,6 @@ static unsigned long change_pte_range(struct
> > vm_area_struct *vma, pmd_t *pmd,
> > unsigned long addr, unsigned long end, pgprot_t newprot,
> > int dirty_accountable, int prot_numa)
> > {
> > - struct mm_struct *mm = vma->vm_mm;
> > pte_t *pte, oldpte;
> > spinlock_t *ptl;
> > unsigned long pages = 0;
> >
>
>
Oops, I blindly assumed the compiler is right, sorry for that. GCC
complains it's unused when building usermode linux. I guess it's a
matter of how set_pte_at() is defined for ARCH=um. I'll take a second
look.
Bart
Re: [RFC PATCH 1/2] Add support of imx7ulp to interconnect framework
Hi Georgi, Sorry for the late response, I have just seen today that you have reviewed my patch. On 1/21/19 6:41 PM, Georgi Djakov wrote: Thank you for working on this! I am expecting the next version. I'm going to send a new patchset soon. I have rewritten pretty much everything, to handle some hardware limitations, and to be more generic. Anyways, most of your comments still makes sense for the new patchset so I will fix them before to submit it. BR, Georgi Best Regards, Alexandre
Re: [LKP] [btrfs] 44fe89de7d: aim7.jobs-per-min -15.1% regression
On 2019/3/12 下午9:50, kernel test robot wrote:
> Greeting,
>
> FYI, we noticed a -15.1% regression of aim7.jobs-per-min due to commit:
>
>
> commit: 44fe89de7d5157a4f31f13d94802c7619e23f462 ("btrfs: Do mandatory tree
> block check before submitting bio")
That commit will cause extra check before writing tree block back onto disk.
It's expected to cause regression for metadata heavy workload.
I'm more interesting if there is any new real world performance
regression like database or other more dedicated workload.
Thanks,
Qu
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master
>
> in testcase: aim7
> on test machine: 72 threads Intel(R) Xeon(R) Gold 6139 CPU @ 2.30GHz with
> 128G memory
> with following parameters:
>
> disk: 4BRD_12G
> md: RAID0
> fs: btrfs
> test: sync_disk_rw
> load: 20
> cpufreq_governor: performance
>
> test-description: AIM7 is a traditional UNIX system level benchmark suite
> which is used to test and measure the performance of multiuser system.
> test-url: https://sourceforge.net/projects/aimbench/files/aim-suite7/
>
> In addition to that, the commit also has significant impact on the following
> tests:
>
> +--+---+
> | testcase: change | aim7: aim7.jobs-per-min -20.8% regression
> |
> | test machine | 40 threads Intel(R) Xeon(R) CPU E5-2690 v2 @ 3.00GHz
> with 384G memory |
> | test parameters | cpufreq_governor=performance
> |
> | | disk=4BRD_12G
> |
> | | fs=btrfs
> |
> | | load=20
> |
> | | md=RAID0
> |
> | | test=sync_disk_rw
> |
> +--+---+
>
>
> Details are as below:
> -->
>
>
> To reproduce:
>
> git clone https://github.com/intel/lkp-tests.git
> cd lkp-tests
> bin/lkp install job.yaml # job file is attached in this email
> bin/lkp run job.yaml
>
> =
> compiler/cpufreq_governor/disk/fs/kconfig/load/md/rootfs/tbox_group/test/testcase:
>
> gcc-8/performance/4BRD_12G/btrfs/x86_64-rhel-7.6/20/RAID0/debian-x86_64-2018-04-03.cgz/lkp-skl-2sp7/sync_disk_rw/aim7
>
> commit:
> a92e9c3a3c ("btrfs: extent_io: Handle error better in extent_writepages()")
> 44fe89de7d ("btrfs: Do mandatory tree block check before submitting bio")
>
> a92e9c3a3cef4c0c 44fe89de7d5157a4f31f13d9480
> ---
>fail:runs %reproductionfail:runs
>| | |
>:4 50% 2:4
> dmesg.WARNING:at#for_ip_interrupt_entry/0x
> %stddev %change %stddev
> \ |\
> 1204 -15.1% 1022aim7.jobs-per-min
> 99.66 +18.3% 117.93aim7.time.elapsed_time
> 99.66 +18.3% 117.93aim7.time.elapsed_time.max
> 3014 ± 2% -3.1% 2921aim7.time.minor_page_faults
> 1.236e+09 +44.6% 1.787e+09 ± 27% cpuidle.C1.time
> 90473 +15.8% 104758 ± 2% meminfo.AnonHugePages
> 102742 -16.5% 85841 ± 2% meminfo.max_used_kB
> 11.68-1.89.85mpstat.cpu.-1.sys%
> 0.07 ± 7% -0.00.05 ± 17% mpstat.cpu.-1.usr%
> 664.25 ± 4% +18.1% 784.25 ± 6%
> slabinfo.dmaengine-unmap-16.active_objs
> 664.25 ± 4% +18.1% 784.25 ± 6%
> slabinfo.dmaengine-unmap-16.num_objs
> 88.00+2.3% 90.00vmstat.cpu.id
> 346712 -15.5% 293086vmstat.io.bo
> 558192 -15.4% 472096vmstat.system.cs
> 88.42+2.0% 90.19iostat.cpu.idle
> 11.50 -15.3% 9.74iostat.cpu.system
> 19892 -15.2% 16877iostat.md0.w/s
> 451087 -15.4% 381436iostat.md0.wkB/s
> 252.75 ± 2% -19.8% 202.67 ± 16%
> sched_debug.cfs_rq:/.removed.util_avg.max
> 57.62 ± 2% +71.4% 98.79 ± 51% sched_debug.cpu.cpu_load[1].max
> 47.75 ± 13% +81.6% 86.71 ± 43% sched_debug.cpu.cpu_load[2].max
> 7.35 ± 13% -16.4% 6.14 ± 4% sched_debug.cpu.cpu_load[4].avg
> 422.75
Re:Re: [PATCH] irqchip/gic: fix passing wrong start irq number to irq_alloc_descs() for secondary GICs
Hi, Marc
Thanks for your reply！
At 2019-03-11 23:55:11, "Marc Zyngier" wrote:
>On 11/03/2019 14:52, Liu Xiang wrote:
>> For secondary GICs, the start irq number should skip over SGIs
>> and PPIs. Its value should be 32. So we should pass hwirq_base to
>> irq_alloc_descs() rather than a constant number 16.
>>
>> Signed-off-by: Liu Xiang
>> ---
>> drivers/irqchip/irq-gic.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>> index ba2a37a..351f576 100644
>> --- a/drivers/irqchip/irq-gic.c
>> +++ b/drivers/irqchip/irq-gic.c
>> @@ -1157,7 +1157,7 @@ static int gic_init_bases(struct gic_chip_data *gic,
>> int irq_start,
>>
>> gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
>>
>> -irq_base = irq_alloc_descs(irq_start, 16, gic_irqs,
>> +irq_base = irq_alloc_descs(irq_start, hwirq_base, gic_irqs,
>> numa_node_id());
>> if (irq_base < 0) {
>> WARN(1, "Cannot allocate irq_descs @ IRQ%d, assuming
>> pre-allocated\n",
>>
>
>I suggest you look at __irq_alloc_descs(), and understand what the
>various parameters mean. What you're doing here has absolutely no
>effect.
>
>The right thing to do would be to get rid of this altogether, except
>that we have exactly *one* platform in the tree that is still non-DT
>(some unmaintained Cavium piece of junk). But we can still simplify it,
>as this guy doesn't have a secondary GIC (it is braindead enough).
>
>What I'm suggesting instead is:
>
>From b41fdc4a7bf9045e4871c5b15905ea732ffd044f Mon Sep 17 00:00:00 2001
>From: Marc Zyngier
>Date: Mon, 11 Mar 2019 15:38:10 +
>Subject: [PATCH] irqchip/gic: Drop support for secondary GIC in non-DT systems
>
>We do not have any in-tree platform with this pathological setup,
>and only a single system (Cavium's cns3xxx) isn't DT aware.
>
>Let's drop the secondary GIC support for now, until we remove
>the above horror altogether.
>
>Signed-off-by: Marc Zyngier
>---
> arch/arm/mach-cns3xxx/core.c| 2 +-
> drivers/irqchip/irq-gic.c | 45 -
> include/linux/irqchip/arm-gic.h | 3 +--
> 3 files changed, 18 insertions(+), 32 deletions(-)
>
>diff --git a/arch/arm/mach-cns3xxx/core.c b/arch/arm/mach-cns3xxx/core.c
>index 7d5a44a06648..f676592d8402 100644
>--- a/arch/arm/mach-cns3xxx/core.c
>+++ b/arch/arm/mach-cns3xxx/core.c
>@@ -90,7 +90,7 @@ void __init cns3xxx_map_io(void)
> /* used by entry-macro.S */
> void __init cns3xxx_init_irq(void)
> {
>- gic_init(0, 29, IOMEM(CNS3XXX_TC11MP_GIC_DIST_BASE_VIRT),
>+ gic_init(IOMEM(CNS3XXX_TC11MP_GIC_DIST_BASE_VIRT),
>IOMEM(CNS3XXX_TC11MP_GIC_CPU_BASE_VIRT));
> }
>
>diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>index ba2a37a27a54..fd3110c171ba 100644
>--- a/drivers/irqchip/irq-gic.c
>+++ b/drivers/irqchip/irq-gic.c
>@@ -1089,11 +1089,10 @@ static void gic_init_chip(struct gic_chip_data *gic,
>struct device *dev,
> #endif
> }
>
>-static int gic_init_bases(struct gic_chip_data *gic, int irq_start,
>+static int gic_init_bases(struct gic_chip_data *gic,
> struct fwnode_handle *handle)
> {
>- irq_hw_number_t hwirq_base;
>- int gic_irqs, irq_base, ret;
>+ int gic_irqs, ret;
>
> if (IS_ENABLED(CONFIG_GIC_NON_BANKED) && gic->percpu_offset) {
> /* Frankein-GIC without banked registers... */
>@@ -1145,28 +1144,21 @@ static int gic_init_bases(struct gic_chip_data *gic,
>int irq_start,
> } else {/* Legacy support */
> /*
>* For primary GICs, skip over SGIs.
>- * For secondary GICs, skip over PPIs, too.
>+ * No secondary GIC support whatsoever.
>*/
>- if (gic == _data[0] && (irq_start & 31) > 0) {
>- hwirq_base = 16;
>- if (irq_start != -1)
>- irq_start = (irq_start & ~31) + 16;
>- } else {
>- hwirq_base = 32;
>- }
>+ int irq_base;
>
>- gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
>+ gic_irqs -= 16; /* calculate # of irqs to allocate */
>
>- irq_base = irq_alloc_descs(irq_start, 16, gic_irqs,
>+ irq_base = irq_alloc_descs(16, 16, gic_irqs,
> numa_node_id());
> if (irq_base < 0) {
>- WARN(1, "Cannot allocate irq_descs @ IRQ%d, assuming
>pre-allocated\n",
>- irq_start);
>- irq_base = irq_start;
>+ WARN(1, "Cannot allocate irq_descs @ IRQ16, assuming
>pre-allocated\n");
>+ irq_base = 16;
> }
>
> gic->domain = irq_domain_add_legacy(NULL, gic_irqs,
Re: [PATCH] mm: remove unused variable
On 3/12/19 7:28 AM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski
>
> The mm variable is set but unused. Remove it.
It is used. Look further down for calls to set_pte_at().
--
Khalid
>
> Signed-off-by: Bartosz Golaszewski
> ---
> mm/mprotect.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/mm/mprotect.c b/mm/mprotect.c
> index 028c724dcb1a..130dac3ad04f 100644
> --- a/mm/mprotect.c
> +++ b/mm/mprotect.c
> @@ -39,7 +39,6 @@ static unsigned long change_pte_range(struct vm_area_struct
> *vma, pmd_t *pmd,
> unsigned long addr, unsigned long end, pgprot_t newprot,
> int dirty_accountable, int prot_numa)
> {
> - struct mm_struct *mm = vma->vm_mm;
> pte_t *pte, oldpte;
> spinlock_t *ptl;
> unsigned long pages = 0;
>
Re: [RFC] sched/fair: hard lockup in sched_cfs_period_timer
On Mon, Mar 11, 2019 at 04:25:36PM -0400 Phil Auld wrote:
> On Mon, Mar 11, 2019 at 10:44:25AM -0700 bseg...@google.com wrote:
> > Letting it spin for 100ms and then only increasing by 6% seems extremely
> > generous. If we went this route I'd probably say "after looping N
> > times, set the period to time taken / N + X%" where N is like 8 or
> > something. I think I'd probably perfer something like this to the
> > previous "just abort and let it happen again next interrupt" one.
>
> Okay. I'll try to spin something up that does this. It may be a little
> trickier to keep the quota proportional to the new period. I think that's
> important since we'll be changing the user's setting.
>
> Do you mean to have it break when it hits N and recalculates the period or
> reset the counter and keep going?
>
Let me know what you think of the below. It's working nicely. I like your
suggestion to limit it quickly based on number of loops and use that to
scale up. I think it is best to break out and let it fire again if needed.
The warning fires once, very occasionally twice, and then things are quiet.
If that looks reasonable I'll do some more testing and spin it up as a real
patch submission.
Cheers,
Phil
---
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 310d0637fe4b..54b30adfc89e 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -4859,19 +4859,51 @@ static enum hrtimer_restart
sched_cfs_slack_timer(struct hrtimer *timer)
return HRTIMER_NORESTART;
}
+extern const u64 max_cfs_quota_period;
+int cfs_period_autotune_loop_limit = 8;
+int cfs_period_autotune_cushion_pct = 15; /* percentage added to period
recalculation */
+
static enum hrtimer_restart sched_cfs_period_timer(struct hrtimer *timer)
{
struct cfs_bandwidth *cfs_b =
container_of(timer, struct cfs_bandwidth, period_timer);
+ s64 nsstart, nsnow, new_period;
int overrun;
int idle = 0;
+ int count = 0;
raw_spin_lock(_b->lock);
+ nsstart = ktime_to_ns(hrtimer_cb_get_time(timer));
for (;;) {
overrun = hrtimer_forward_now(timer, cfs_b->period);
if (!overrun)
break;
+ if (++count > cfs_period_autotune_loop_limit) {
+ ktime_t old_period = ktime_to_ns(cfs_b->period);
+
+ nsnow = ktime_to_ns(hrtimer_cb_get_time(timer));
+ new_period = (nsnow -
nsstart)/cfs_period_autotune_loop_limit;
+
+ /* Make sure new period will be larger than old. */
+ if (new_period < old_period) {
+ new_period = old_period;
+ }
+ new_period += (new_period *
cfs_period_autotune_cushion_pct) / 100;
+
+ if (new_period > max_cfs_quota_period)
+ new_period = max_cfs_quota_period;
+
+ cfs_b->period = ns_to_ktime(new_period);
+ cfs_b->quota += (cfs_b->quota * ((new_period -
old_period) * 100)/old_period)/100;
+ pr_warn_ratelimited(
+ "cfs_period_timer[cpu%d]: period too short,
scaling up (new cfs_period_us %lld, cfs_quota_us = %lld)\n",
+ smp_processor_id(),
cfs_b->period/NSEC_PER_USEC, cfs_b->quota/NSEC_PER_USEC);
+
+ idle = 0;
+ break;
+ }
+
idle = do_sched_cfs_period_timer(cfs_b, overrun);
}
if (idle)
--
Re: [PATCH 1/3] userfaultfd/sysctl: introduce unprivileged_userfaultfd
On Tue, Mar 12, 2019 at 08:26:33PM +0800, Peter Xu wrote: > On Tue, Mar 12, 2019 at 08:58:30AM +0200, Mike Rapoport wrote: > > [...] > > > > +config USERFAULTFD_UNPRIVILEGED_DEFAULT > > > +string "Default behavior for unprivileged userfault syscalls" > > > +depends on USERFAULTFD > > > +default "disabled" > > > +help > > > + Set this to "enabled" to allow userfaultfd syscalls from > > > + unprivileged users. Set this to "disabled" to forbid > > > + userfaultfd syscalls from unprivileged users. Set this to > > > + "kvm" to forbid unpriviledged users but still allow users > > > + who had enough permission to open /dev/kvm. > > > > I'd phrase it a bit differently: > > > > This option controls privilege level required to execute userfaultfd > ^ > + add " the default"? > > > system call. > > > > Set this to "enabled" to allow userfaultfd system call from unprivileged > > users. > > Set this to "disabled" to allow userfaultfd system call only for users who > > have ptrace capability. > > Set this to "kvm" to restrict userfaultfd system call usage to users with > ^ > add " who have ptrace capability, or" ---+ > > > permissions to open "/dev/kvm". > > I think your version is better than mine, but I'd like to confirm > about above two extra changes before I squash them into the patch. :) I like your changes. > Thanks! > > -- > Peter Xu > -- Sincerely yours, Mike.
[GIT PULL RESEND] pidfd changes for v5.1-rc1
Hi Linus, This is a resend of the pull request for the pidfd_send_signal() syscall which I sent last Tuesday. I'm not sure whether you just wanted to take a closer look. The following changes since commit f17b5f06cb92ef2250513a1e154c47b78df07d40: Linux 5.0-rc4 (2019-01-27 15:18:05 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git tags/pidfd-v5.1-rc1 The patchset introduces the ability to use file descriptors from proc/ as stable handles on struct pid. Even if a pid is recycled the handle will not change. For a start these fds can be used to send signals to the processes they refer to. With the ability to use /proc/ fds as stable handles on struct pid we can fix a long-standing issue where after a process has exited its pid can be reused by another process. If a caller sends a signal to a reused pid it will end up signaling the wrong process. With this patchset we enable a variety of use cases. One obvious example is that we can now safely delegate an important part of process management - sending signals - to processes other than the parent of a given process by sending file descriptors around via scm rights and not fearing that the given process will have been recycled in the meantime. It also allows for easy testing whether a given process is still alive or not by sending signal 0 to a pidfd which is quite handy. There has been some interest in this feature e.g. from systems management (systemd, glibc) and container managers. I have requested and gotten comments from glibc to make sure that this syscall is suitable for their needs as well. In the future I expect it to take on most other pid-based signal syscalls. But such features are left for the future once they are needed. The patchset has been sitting in linux-next for quite a while and has not caused any issues. It comes with selftests which verify basic functionality and also test that a recycled pid cannot be signaled via a pidfd. Jon has written about a prior version of this patchset. It should cover the basic functionality since not a lot has changed since then: https://lwn.net/Articles/773459/ The commit message for the syscall itself is extensively documenting the syscall, including it's functionality and extensibility. /* Merge conflict and sycall number coordination */ Please note, there will be a merge conflict between the Jens' io_uring patch set in the block tree and this tree. To minimize its impact Arnd worked with Jens and me to coordinate syscall numbers in advance. pidfd_send_signal() takes 424 and Jens' patchset took 425 to 427. /* Separate tree on kernel.org */ At the beginning of last merge cycle it was suggested to move this patchset into a separate tree on kernel.org as there will be more work coming that will be extending the use of file descriptors for processes. The tree was announced in January: https://lore.kernel.org/lkml/20190108234722.bojj5bqowluty...@brauner.io/ The pidfd tree is located on kernel.org https://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git/ and it's for-next branch is already tracked by Stephen in linux-next since the beginning of the 5.0 development cycle. I'm prepared to deal with any fallouts coming from this work going forward. The only thing that has changed recently in these patches was the addition of two more Acked-by/Reviewed-by from David Howells and tglx after the last round of reviews. Please consider pulling these changes from the signed pidfd-v5.1-rc1 tag. Thanks! Christian pidfd patches for v5.1-rc1 Christian Brauner (2): signal: add pidfd_send_signal() syscall selftests: add tests for pidfd_send_signal() arch/x86/entry/syscalls/syscall_32.tbl | 1 + arch/x86/entry/syscalls/syscall_64.tbl | 1 + fs/proc/base.c | 9 + include/linux/proc_fs.h| 6 + include/linux/syscalls.h | 3 + include/uapi/asm-generic/unistd.h | 4 +- kernel/signal.c| 133 +- kernel/sys_ni.c| 1 + tools/testing/selftests/Makefile | 1 + tools/testing/selftests/pidfd/Makefile | 6 + tools/testing/selftests/pidfd/pidfd_test.c | 381 + 11 files changed, 539 insertions(+), 7 deletions(-) create mode 100644 tools/testing/selftests/pidfd/Makefile create mode 100644 tools/testing/selftests/pidfd/pidfd_test.c
Re: [PATCH 3/3] perf, tools, report: Set up samples correctly in hierarchy mode
On Tue, Mar 12, 2019 at 12:12:59PM +0100, Jiri Olsa wrote:
> On Mon, Mar 11, 2019 at 08:52:24PM -0700, Andi Kleen wrote:
> > From: Andi Kleen
> >
> > In hierarchy mode the res samples need to be cloned from the parent
> > entry. Copy them in this case. This fixes res sample browsing
> > with --hierarchy.
> >
> > Signed-off-by: Andi Kleen
> > ---
> > tools/perf/util/hist.c | 11 +--
> > 1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> > index 1f230285d78a..dcf24581dfbd 100644
> > --- a/tools/perf/util/hist.c
> > +++ b/tools/perf/util/hist.c
> > @@ -437,8 +437,15 @@ static int hist_entry__init(struct hist_entry *he,
> > }
> >
> > if (symbol_conf.res_sample) {
> > - he->res_samples = calloc(sizeof(struct res_sample),
> > - symbol_conf.res_sample);
> > + if (he->res_samples) {
>
> I dont think this leg will ever execute because we don't set
> res_samples in template
I originally thought this too, but ...
The hierarchy mode calls it with a hist entry that is not
a template.
-Andi
Re: [PATCH v1 1/4] auxdisplay: hd44780: Fix memory leak on ->remove()
Hi Andy,
On Tue, Mar 12, 2019 at 2:18 PM Andy Shevchenko
wrote:
> We have to free on ->remove() the allocated resources on ->probe().
>
> Fixes: d47d88361fee ("auxdisplay: Add HD44780 Character LCD support")
> Cc: Geert Uytterhoeven
> Signed-off-by: Andy Shevchenko
Thanks, nice catch!
> --- a/drivers/auxdisplay/hd44780.c
> +++ b/drivers/auxdisplay/hd44780.c
> @@ -280,6 +280,8 @@ static int hd44780_remove(struct platform_device *pdev)
> struct charlcd *lcd = platform_get_drvdata(pdev);
>
> charlcd_unregister(lcd);
> +
> + kfree(lcd);
(wearing a newer (and hopefully wiser ;-) hat than when I wrote the code)
While this is correct for the current implementation of struct charlcd_priv,
this may be a bit fragile.
What about adding a charlcd_free() wrapper, which can do kfree(to_priv(lcd)),
and be used in the probe failure path, too?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: KASAN: null-ptr-deref Read in reclaim_high
On Tue, Mar 12, 2019 at 1:33 AM Dmitry Vyukov wrote:
>
> On Tue, Mar 12, 2019 at 7:25 AM Andrew Morton
> wrote:
> >
> > On Tue, 12 Mar 2019 07:08:38 +0100 Dmitry Vyukov wrote:
> >
> > > On Tue, Mar 12, 2019 at 12:37 AM Andrew Morton
> > > wrote:
> > > >
> > > > On Mon, 11 Mar 2019 06:08:01 -0700 syzbot
> > > > wrote:
> > > >
> > > > > syzbot has bisected this bug to:
> > > > >
> > > > > commit 29a4b8e275d1f10c51c7891362877ef6cffae9e7
> > > > > Author: Shakeel Butt
> > > > > Date: Wed Jan 9 22:02:21 2019 +
> > > > >
> > > > > memcg: schedule high reclaim for remote memcgs on high_work
> > > > >
> > > > > bisection log:
> > > > > https://syzkaller.appspot.com/x/bisect.txt?x=155bf5db20
> > > > > start commit: 29a4b8e2 memcg: schedule high reclaim for remote
> > > > > memcgs on..
> > > > > git tree: linux-next
> > > > > final crash:
> > > > > https://syzkaller.appspot.com/x/report.txt?x=175bf5db20
> > > > > console output:
> > > > > https://syzkaller.appspot.com/x/log.txt?x=135bf5db20
> > > > > kernel config:
> > > > > https://syzkaller.appspot.com/x/.config?x=611f89e5b6868db
> > > > > dashboard link:
> > > > > https://syzkaller.appspot.com/bug?extid=fa11f9da42b46cea3b4a
> > > > > userspace arch: amd64
> > > > > syz repro:
> > > > > https://syzkaller.appspot.com/x/repro.syz?x=1425901740
> > > > > C reproducer:
> > > > > https://syzkaller.appspot.com/x/repro.c?x=141630a0c0
> > > > >
> > > > > Reported-by: syzbot+fa11f9da42b46cea3...@syzkaller.appspotmail.com
> > > > > Fixes: 29a4b8e2 ("memcg: schedule high reclaim for remote memcgs on
> > > > > high_work")
> > > >
> > > > The following patch
> > > > memcg-schedule-high-reclaim-for-remote-memcgs-on-high_work-v3.patch
> > > > might have fixed this. Was it applied?
> > >
> > > Hi Andrew,
> > >
> > > You mean if the patch was applied during the bisection?
> > > No, it wasn't. Bisection is very specifically done on the same tree
> > > where the bug was hit. There are already too many factors that make
> > > the result flaky/wrong/inconclusive without changing the tree state.
> > > Now, if syzbot would know about any pending fix for this bug, then it
> > > would not do the bisection at all. But it have not seen any patch in
> > > upstream/linux-next with the Reported-by tag, nor it received any syz
> > > fix commands for this bugs. Should have been it aware of the fix? How?
> >
> > memcg-schedule-high-reclaim-for-remote-memcgs-on-high_work-v3.patch was
> > added to linux-next on Jan 10. I take it that this bug was hit when
> > testing the entire linux-next tree, so we can assume that
> > memcg-schedule-high-reclaim-for-remote-memcgs-on-high_work-v3.patch
> > does not fix it, correct?
> > In which case, over to Shakeel!
>
> Jan 10 is exactly when this bug was reported:
> https://groups.google.com/forum/#!msg/syzkaller-bugs/5YkhNUg2PFY/4-B5M7bDCAAJ
> https://syzkaller.appspot.com/bug?extid=fa11f9da42b46cea3b4a
>
> We don't know if that patch fixed the bug or not because nobody tested
> the reproducer with that patch.
>
> It seems that the problem here is that nobody associated the fix with
> the bug report. So people looking at open bug reports will spend time
> again and again debugging this just to find that this was fixed months
> ago. syzbot also doesn't have a chance to realize that this is fixed
> and bisection is not necessary anymore. It also won't confirm/disprove
> that the fix actually fixes the bug because even if the crash will
> continue to happen it will look like the old crash just continues to
> happen, so nothing to notify about.
>
> Associating fixes with bug reports solves all these problems for
> humans and bots.
Should we add "Reported-by" for syzbot reports on linux-next patches
as well? Please note that these patches are in flux and might be
dropped or completely changed before merging into Linus tree.
Also should syzbot drop bug reports on older linux-next trees if it
can not be repro'ed in the latest linux-next tree? IMHO yes.
Shakeel
Re: [PATCH] drivers: gpio: octeon: use devm_platform_ioremap_resource()
pon., 11 mar 2019 o 20:48 Enrico Weigelt, metux IT consult
napisał(a):
>
> Use the new helper that wraps the calls to platform_get_resource()
> and devm_ioremap_resource() together.
>
> Signed-off-by: Enrico Weigelt, metux IT consult
> ---
> drivers/gpio/gpio-octeon.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/gpio/gpio-octeon.c b/drivers/gpio/gpio-octeon.c
> index 1b19c88..afb0e8a 100644
> --- a/drivers/gpio/gpio-octeon.c
> +++ b/drivers/gpio/gpio-octeon.c
> @@ -82,7 +82,6 @@ static int octeon_gpio_probe(struct platform_device *pdev)
> {
> struct octeon_gpio *gpio;
> struct gpio_chip *chip;
> - struct resource *res_mem;
> void __iomem *reg_base;
> int err = 0;
>
> @@ -91,8 +90,7 @@ static int octeon_gpio_probe(struct platform_device *pdev)
> return -ENOMEM;
> chip = >chip;
>
> - res_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - reg_base = devm_ioremap_resource(>dev, res_mem);
> + reg_base = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(reg_base))
> return PTR_ERR(reg_base);
>
> --
> 1.9.1
>
Can you make this a part of the bigger series and resend together with
subject line fixes?
Also: maybe consider adding a coccinelle script for that. When I added
that function I noticed there are 1200+ instances in the kernel that
need fixing. I think we'll be better off automating it.
Bart
Re: [PATCHv7 10/10] doc/mm: New documentation for memory performance
On Mon, 11 Mar 2019 14:16:33 -0600 Keith Busch wrote: > On Mon, Mar 11, 2019 at 04:38:43AM -0700, Jonathan Cameron wrote: > > On Wed, 27 Feb 2019 15:50:38 -0700 > > Keith Busch wrote: > > > > > Platforms may provide system memory where some physical address ranges > > > perform differently than others, or is side cached by the system. > > The magic 'side cached' term still here in the patch description, ideally > > wants cleaning up. > > > > > > > > Add documentation describing a high level overview of such systems and the > > > perforamnce and caching attributes the kernel provides for applications > > performance > > > > > wishing to query this information. > > > > > > Reviewed-by: Mike Rapoport > > > Signed-off-by: Keith Busch > > > > A few comments inline. Mostly the weird corner cases that I miss understood > > in one of the earlier versions of the code. > > > > Whilst I think perhaps that one section could be tweaked a tiny bit I'm > > basically > > happy with this if you don't want to. > > > > Reviewed-by: Jonathan Cameron > > > > > --- > > > Documentation/admin-guide/mm/numaperf.rst | 164 > > > ++ > > > 1 file changed, 164 insertions(+) > > > create mode 100644 Documentation/admin-guide/mm/numaperf.rst > > > > > > diff --git a/Documentation/admin-guide/mm/numaperf.rst > > > b/Documentation/admin-guide/mm/numaperf.rst > > > new file mode 100644 > > > index ..d32756b9be48 > > > --- /dev/null > > > +++ b/Documentation/admin-guide/mm/numaperf.rst > > > @@ -0,0 +1,164 @@ > > > +.. _numaperf: > > > + > > > += > > > +NUMA Locality > > > += > > > + > > > +Some platforms may have multiple types of memory attached to a compute > > > +node. These disparate memory ranges may share some characteristics, such > > > +as CPU cache coherence, but may have different performance. For example, > > > +different media types and buses affect bandwidth and latency. > > > + > > > +A system supports such heterogeneous memory by grouping each memory type > > > +under different domains, or "nodes", based on locality and performance > > > +characteristics. Some memory may share the same node as a CPU, and > > > others > > > +are provided as memory only nodes. While memory only nodes do not provide > > > +CPUs, they may still be local to one or more compute nodes relative to > > > +other nodes. The following diagram shows one such example of two compute > > > +nodes with local memory and a memory only node for each of compute node: > > > + > > > + +--+ +--+ > > > + | Compute Node 0 +-+ Compute Node 1 | > > > + | Local Node0 Mem | | Local Node1 Mem | > > > + ++-+ ++-+ > > > + || > > > + ++-+ ++-+ > > > + | Slower Node2 Mem | | Slower Node3 Mem | > > > + +--+ ++-+ > > > + > > > +A "memory initiator" is a node containing one or more devices such as > > > +CPUs or separate memory I/O devices that can initiate memory requests. > > > +A "memory target" is a node containing one or more physical address > > > +ranges accessible from one or more memory initiators. > > > + > > > +When multiple memory initiators exist, they may not all have the same > > > +performance when accessing a given memory target. Each initiator-target > > > +pair may be organized into different ranked access classes to represent > > > +this relationship. > > > > This concept is a bit vague at the moment. Largely because only access0 > > is actually defined. We should definitely keep a close eye on any others > > that are defined in future to make sure this text is still valid. > > > > I can certainly see it being used for different ideas of 'best' rather > > than simply best and second best etc. > > I tried to make the interface flexible to future extension, but I'm > still not sure how potential users would want to see something like > all pair-wise attributes, so I had some trouble trying to capture that > in words. Agreed, it is definitely non obvious. We might end up with something totally different like Jerome is proposing anyway. Let's address this when it happens! > > > > The highest performing initiator to a given target > > > +is considered to be one of that target's local initiators, and given > > > +the highest access class, 0. Any given target may have one or more > > > +local initiators, and any given initiator may have multiple local > > > +memory targets. > > > + > > > +To aid applications matching memory targets with their initiators, the > > > +kernel provides symlinks to each other. The following example lists the > > > +relationship for the access class "0" memory initiators and targets, > > > which is > > > +the of nodes with the highest performing access relationship:: > > > + > > > + # symlinks -v
Re: [PATCH] mm: vmscan: show zone type in kswapd tracepoints
On Tue 12-03-19 19:04:43, Yafang Shao wrote: > On Mon, Mar 11, 2019 at 4:47 PM Michal Hocko wrote: > > > > On Fri 01-03-19 15:38:54, Yafang Shao wrote: > > > If we want to know the zone type, we have to check whether > > > CONFIG_ZONE_DMA, CONFIG_ZONE_DMA32 and CONFIG_HIGHMEM are set or not, > > > that's not so convenient. > > > > > > We'd better show the zone type directly. > > > > I do agree that zone number is quite PITA to process in general but do > > we really need this information in the first place? Why do we even care? > > > > Sometimes we want to know this event occurs in which zone, then we can > get the information of this zone, > for example via /proc/zoneinfo. > It could give us more information for debugging. Could you be more specific please? > > Zones are an MM internal implementation details and the more we export > > to the userspace the more we are going to argue about breaking userspace > > when touching them. So I would rather not export that information unless > > it is terribly useful. > > > > I 'm not sure whether zone type is terribly useful or not, but the > 'zid' is useless at all. > > I don't agree that Zones are MM internal. > We can get the zone type in many ways, for example /proc/zoneinfo. > > If we show this event occurs in which zone, we'd better show the zone type, > or we should drop this 'zid'. Yes, I am suggesting the later. If somebody really needs it then I would like to see a _specific_ usecase. Then we can add the proper name. -- Michal Hocko SUSE Labs
Re: [PATCH] ata: pata_oldpiix: Add missing device ID for INTEL_82371AB
On Tue, Mar 12, 2019 at 12:06:23PM +, Alan Cox wrote: > On Tue, 12 Mar 2019 11:41:02 +0100 > LABBE Corentin wrote: > > > On Mon, Dec 10, 2018 at 05:52:35PM +0300, Sergei Shtylyov wrote: > > > Hello! > > > > > > On 12/10/2018 04:46 PM, Corentin Labbe wrote: > > > > > > > When playing with a virtual SPARC machine with qemu, I found that the > > > > IDE emulated device was not probing with the ata/pata_oldpiix driver. > > > > > >Correctly, it should probe with ata_piix, > > > > > > > But with the old ide/piix, it was probed.> > > > > This is due to this PCI devid was not migrated from the old ide/piix. > > > > > >It wasn't on purpose -- the IDE driver supports the original PIIX > > > incorrectly. > > > > > > > Hello > > > > What about removing this old driver totally if it dont work ? > > If the virtual Sparc emulator is using it does that also mean actual > Sparc hardware has it. In which case presumably it needs fixing, or at > least moving to the generic driver assuming the firmware sets it up ? > The qemu works perfectly with the new Linux driver.
Re: [PATCHv4 26/28] PCI: mobiveil: ls_pcie_g4: add Workaround for A-011451
On Tue, Mar 12, 2019 at 09:34:17AM +, Z.q. Hou wrote:
> Hi Bjorn,
>
> Thanks a lot for your comments!
>
> > -Original Message-
> > From: Bjorn Helgaas [mailto:helg...@kernel.org]
> > Sent: 2019年3月12日 1:35
> > To: Z.q. Hou
> > Cc: linux-...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> > devicet...@vger.kernel.org; linux-kernel@vger.kernel.org;
> > robh...@kernel.org; mark.rutl...@arm.com; l.subrahma...@mobiveil.co.in;
> > shawn...@kernel.org; Leo Li ;
> > lorenzo.pieral...@arm.com; catalin.mari...@arm.com;
> > will.dea...@arm.com; Mingkai Hu ; M.h. Lian
> > ; Xiaowei Bao
> > Subject: Re: [PATCHv4 26/28] PCI: mobiveil: ls_pcie_g4: add Workaround for
> > A-011451
> >
> > On Mon, Mar 11, 2019 at 09:33:32AM +, Z.q. Hou wrote:
> > > From: Hou Zhiqiang
> > >
> > > When LX2 PCIe controller is sending multiple split completions and ACK
> > > latency expires indicating that ACK should be send at priority.
> > > But because of large number of split completions and FC update DLLP,
> > > the controller does not give priority to ACK transmission. This
> > > results into ACK latency timer timeout error at the link partner and
> > > the pending TLPs are replayed by the link partner again.
> > >
> > > Workaround:
> > > 1. Reduce the ACK latency timeout value to a very small value.
> > > 2. Restrict the number of completions from the LX2 PCIe controller
> > >to 1, by changing the Max Read Request Size (MRRS) of link partner
> > >to the same value as Max Packet size (MPS).
> > >
> > > This patch implemented part 1, the part 2 can be set by kernel
> > > parameter 'pci=pcie_bus_perf'
> >
> > If I understand correctly, you're saying that LX2160A Rev1.0 will only work
> > correctly if you have this patch applied AND you boot with
> > "pci=pcie_bus_perf".
>
> Your understanding is correct.
>
> > That might be OK if these rev 1.0 parts are only used in the lab and are
> > never
> > shipped to customers. But if these parts are ever shipped to customers, I
> > don't think it's acceptable for them to have to figure out that they must
> > boot
> > with "pci=pcie_bus_perf". Yes, you can document that in release notes, but
> > it's still a poor user experience, and users will forget, and they will see
> > mysterious hard-to-debug issues.
Since you didn't respond here, I assume these rev 1.0 parts have been
or will be shipped to end users? Please confirm.
> > Maybe there's a way for you to automatically set that pcie_bus_perf mode?
> > With a dmesg note to indicate that you're overriding any mode the user may
> > have selected?
>
> Actually we don't have a way to set the pcie_bus_perf automatically
> under Linux, we give this parameter in kernel cmdline. Do you have
> any advice about how to set this parameter automatically under
> Linux?
The beauty of Linux being open-source is that if you need something
that doesn't exist, you are empowered to create it.
We do have one place already (cns3xxx_pcie_hw_init()) that explicitly
sets pcie_bus_config. You might be able to do something similar.
> > > This ERRATA is only for LX2160A Rev1.0, and it will be fixed in
> > > Rev2.0.
> > >
> > > Signed-off-by: Hou Zhiqiang
> > > ---
> > > V4:
> > > - no change
> > >
> > > .../pci/controller/mobiveil/pci-layerscape-gen4.c | 15 +++
> > > drivers/pci/controller/mobiveil/pcie-mobiveil.h | 4
> > > 2 files changed, 19 insertions(+)
> > >
> > > diff --git a/drivers/pci/controller/mobiveil/pci-layerscape-gen4.c
> > > b/drivers/pci/controller/mobiveil/pci-layerscape-gen4.c
> > > index d2c5dbbd5e3c..20ce146788ca 100644
> > > --- a/drivers/pci/controller/mobiveil/pci-layerscape-gen4.c
> > > +++ b/drivers/pci/controller/mobiveil/pci-layerscape-gen4.c
> > > @@ -82,12 +82,27 @@ static bool ls_pcie_g4_is_bridge(struct ls_pcie_g4
> > *pcie)
> > > return header_type == PCI_HEADER_TYPE_BRIDGE; }
> > >
> > > +static void workaround_A011451(struct ls_pcie_g4 *pcie) {
> > > + struct mobiveil_pcie *mv_pci = pcie->pci;
> > > + u32 val;
> > > +
> > > + /* Set ACK latency timeout */
> > > + val = csr_readl(mv_pci, GPEX_ACK_REPLAY_TO);
> > > + val &= ~(ACK_LAT_TO_VAL_MASK << ACK_LAT_TO_VAL_SHIFT);
> > > + val |= (4 << ACK_LAT_TO_VAL_SHIFT);
> > > + csr_writel(mv_pci, val, GPEX_ACK_REPLAY_TO); }
> > > +
> > > static int ls_pcie_g4_host_init(struct mobiveil_pcie *pci) {
> > > struct ls_pcie_g4 *pcie = to_ls_pcie_g4(pci);
> > >
> > > pcie->rev = csr_readb(pci, PCI_REVISION_ID);
> > >
> > > + if (pcie->rev == REV_1_0)
> > > + workaround_A011451(pcie);
> > > +
> > > return 0;
> > > }
> > >
> > > diff --git a/drivers/pci/controller/mobiveil/pcie-mobiveil.h
> > > b/drivers/pci/controller/mobiveil/pcie-mobiveil.h
> > > index ab43de5e4b2b..f0e2e4ae09b5 100644
> > > --- a/drivers/pci/controller/mobiveil/pcie-mobiveil.h
> > > +++ b/drivers/pci/controller/mobiveil/pcie-mobiveil.h
> > > @@ -85,6 +85,10 @@
> > > #define PAB_AXI_AMAP_PEX_WIN_H(win) PAB_REG_ADDR(0x0bac,
> > win)
> >
[PATCH] um: remove unused variable
From: Bartosz Golaszewski
The buf variable is unused. Remove it.
Signed-off-by: Bartosz Golaszewski
---
arch/um/kernel/skas/uaccess.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/um/kernel/skas/uaccess.c b/arch/um/kernel/skas/uaccess.c
index 7f06fdbc7ee1..bd3cb694322c 100644
--- a/arch/um/kernel/skas/uaccess.c
+++ b/arch/um/kernel/skas/uaccess.c
@@ -59,7 +59,6 @@ static pte_t *maybe_map(unsigned long virt, int is_write)
static int do_op_one_page(unsigned long addr, int len, int is_write,
int (*op)(unsigned long addr, int len, void *arg), void *arg)
{
- jmp_buf buf;
struct page *page;
pte_t *pte;
int n;
--
2.20.1
[PATCH] um: remove uses of variable length arrays
From: Bartosz Golaszewski
While the affected code is run in user-mode, the build still warns
about it. Convert all uses of VLA to dynamic allocations.
Signed-off-by: Bartosz Golaszewski
---
arch/um/os-Linux/umid.c | 36 +++-
1 file changed, 27 insertions(+), 9 deletions(-)
diff --git a/arch/um/os-Linux/umid.c b/arch/um/os-Linux/umid.c
index 998fbb445458..e261656fe9d7 100644
--- a/arch/um/os-Linux/umid.c
+++ b/arch/um/os-Linux/umid.c
@@ -135,12 +135,18 @@ static int remove_files_and_dir(char *dir)
*/
static inline int is_umdir_used(char *dir)
{
- char file[strlen(uml_dir) + UMID_LEN + sizeof("/pid\0")];
- char pid[sizeof("n\0")], *end;
+ char pid[sizeof("n\0")], *end, *file;
int dead, fd, p, n, err;
+ size_t filelen;
- n = snprintf(file, sizeof(file), "%s/pid", dir);
- if (n >= sizeof(file)) {
+ err = asprintf(, "%s/pid", dir);
+ if (err < 0)
+ return 0;
+
+ filelen = strlen(file);
+
+ n = snprintf(file, filelen, "%s/pid", dir);
+ if (n >= filelen) {
printk(UM_KERN_ERR "is_umdir_used - pid filename too long\n");
err = -E2BIG;
goto out;
@@ -185,6 +191,7 @@ static inline int is_umdir_used(char *dir)
out_close:
close(fd);
out:
+ free(file);
return 0;
}
@@ -210,18 +217,21 @@ static int umdir_take_if_dead(char *dir)
static void __init create_pid_file(void)
{
- char file[strlen(uml_dir) + UMID_LEN + sizeof("/pid\0")];
- char pid[sizeof("n\0")];
+ char pid[sizeof("n\0")], *file;
int fd, n;
- if (umid_file_name("pid", file, sizeof(file)))
+ file = malloc(strlen(uml_dir) + UMID_LEN + sizeof("/pid\0"));
+ if (!file)
return;
+ if (umid_file_name("pid", file, sizeof(file)))
+ goto out;
+
fd = open(file, O_RDWR | O_CREAT | O_EXCL, 0644);
if (fd < 0) {
printk(UM_KERN_ERR "Open of machine pid file \"%s\" failed: "
"%s\n", file, strerror(errno));
- return;
+ goto out;
}
snprintf(pid, sizeof(pid), "%d\n", getpid());
@@ -231,6 +241,8 @@ static void __init create_pid_file(void)
errno);
close(fd);
+out:
+ free(file);
}
int __init set_umid(char *name)
@@ -385,13 +397,19 @@ __uml_setup("uml_dir=", set_uml_dir,
static void remove_umid_dir(void)
{
- char dir[strlen(uml_dir) + UMID_LEN + 1], err;
+ char *dir, err;
+
+ dir = malloc(strlen(uml_dir) + UMID_LEN + 1);
+ if (!dir)
+ return;
sprintf(dir, "%s%s", uml_dir, umid);
err = remove_files_and_dir(dir);
if (err)
os_warn("%s - remove_files_and_dir failed with err = %d\n",
__func__, err);
+
+ free(dir);
}
__uml_exitcall(remove_umid_dir);
--
2.20.1
[PATCH] mm: remove unused variable
From: Bartosz Golaszewski
The mm variable is set but unused. Remove it.
Signed-off-by: Bartosz Golaszewski
---
mm/mprotect.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/mm/mprotect.c b/mm/mprotect.c
index 028c724dcb1a..130dac3ad04f 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -39,7 +39,6 @@ static unsigned long change_pte_range(struct vm_area_struct
*vma, pmd_t *pmd,
unsigned long addr, unsigned long end, pgprot_t newprot,
int dirty_accountable, int prot_numa)
{
- struct mm_struct *mm = vma->vm_mm;
pte_t *pte, oldpte;
spinlock_t *ptl;
unsigned long pages = 0;
--
2.20.1
Re: [PATCH] ARM: dts: vf610: Add ZII SPB4 board
Hi Andrey,
On Mon, Mar 11, 2019 at 3:49 PM Andrey Smirnov wrote:
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index f4f5aeaf3298..035ad9fc49f3 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -607,7 +607,8 @@ dtb-$(CONFIG_SOC_VF610) += \
> vf610-zii-dev-rev-c.dtb \
> vf610-zii-scu4-aib.dtb \
> vf610-zii-ssmb-dtu.dtb \
> - vf610-zii-ssmb-spu3.dtb
> + vf610-zii-ssmb-spu3.dtb \
> + vf610-zii-spb4.dtb
Please keep it in alphabetical order.
> + gpio-leds {
> + compatible = "gpio-leds";
> + pinctrl-0 = <_leds_debug>;
> + pinctrl-names = "default";
> +
> + led-debug {
> + label = "zii:green:debug1";
> + gpios = < 18 GPIO_ACTIVE_HIGH>;
> + linux,default-trigger = "heartbeat";
> + max-brightness = <1>;
This is an invalid property for gpio-leds. It is used for leds pwm
only. Please remove it.
> + };
> + };
> +
> + reg_vcc_3v3_mcu: regulator {
reg_vcc_3v3_mcu: regulator-vcc-3v3-mcu {
> + {
> + bus-num = <1>;
> + pinctrl-names = "default";
> + pinctrl-0 = <_dspi1>;
> + status = "okay";
> +
> + m25p128@0 {
Node names should be generic:
spi-nor@0
> + {
> + clock-frequency = <10>;
> + pinctrl-names = "default";
> + pinctrl-0 = <_i2c0>;
> + status = "okay";
> +
> + gpio6: pca9505@22 {
io-expander@22
> + compatible = "nxp,pca9554";
> + reg = <0x22>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + };
> +
> + at24c04@50 {
eeprom@50
> + compatible = "atmel,24c04";
> + reg = <0x50>;
> + label = "nameplate";
> + };
> +
> + at24c04@52 {
eeprom@52
[PATCH v1 3/4] auxdisplay: panel: Convert to use charlcd_free()
Convert to use charlcd_free() instead of kfree() for sake of type check.
Signed-off-by: Andy Shevchenko
---
drivers/auxdisplay/panel.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/auxdisplay/panel.c b/drivers/auxdisplay/panel.c
index 21b9b2f2470a..e06de63497cf 100644
--- a/drivers/auxdisplay/panel.c
+++ b/drivers/auxdisplay/panel.c
@@ -1620,7 +1620,7 @@ static void panel_attach(struct parport *port)
if (lcd.enabled)
charlcd_unregister(lcd.charlcd);
err_unreg_device:
- kfree(lcd.charlcd);
+ charlcd_free(lcd.charlcd);
lcd.charlcd = NULL;
parport_unregister_device(pprt);
pprt = NULL;
@@ -1647,7 +1647,7 @@ static void panel_detach(struct parport *port)
if (lcd.enabled) {
charlcd_unregister(lcd.charlcd);
lcd.initialized = false;
- kfree(lcd.charlcd);
+ charlcd_free(lcd.charlcd);
lcd.charlcd = NULL;
}
--
2.20.1
[PATCH v1 2/4] auxdisplay: charlcd: Introduce charlcd_free() helper
The charlcd_free() is a counterpart to charlcd_alloc()
and should be called symmetrically on tear down.
Signed-off-by: Andy Shevchenko
---
drivers/auxdisplay/charlcd.c | 6 ++
include/misc/charlcd.h | 1 +
2 files changed, 7 insertions(+)
diff --git a/drivers/auxdisplay/charlcd.c b/drivers/auxdisplay/charlcd.c
index 60e0b772673f..882be08ddad5 100644
--- a/drivers/auxdisplay/charlcd.c
+++ b/drivers/auxdisplay/charlcd.c
@@ -818,6 +818,12 @@ struct charlcd *charlcd_alloc(unsigned int drvdata_size)
}
EXPORT_SYMBOL_GPL(charlcd_alloc);
+void charlcd_free(struct charlcd *lcd)
+{
+ kfree(lcd);
+}
+EXPORT_SYMBOL_GPL(charlcd_free);
+
static int panel_notify_sys(struct notifier_block *this, unsigned long code,
void *unused)
{
diff --git a/include/misc/charlcd.h b/include/misc/charlcd.h
index 23f61850f363..1832402324ce 100644
--- a/include/misc/charlcd.h
+++ b/include/misc/charlcd.h
@@ -35,6 +35,7 @@ struct charlcd_ops {
};
struct charlcd *charlcd_alloc(unsigned int drvdata_size);
+void charlcd_free(struct charlcd *lcd);
int charlcd_register(struct charlcd *lcd);
int charlcd_unregister(struct charlcd *lcd);
--
2.20.1
[PATCH v1 4/4] auxdisplay: hd44780: Convert to use charlcd_free()
Convert to use charlcd_free() instead of kfree() for sake of type check. Signed-off-by: Andy Shevchenko --- drivers/auxdisplay/hd44780.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/auxdisplay/hd44780.c b/drivers/auxdisplay/hd44780.c index 3cde351fb5c9..ab15b64707ad 100644 --- a/drivers/auxdisplay/hd44780.c +++ b/drivers/auxdisplay/hd44780.c @@ -271,7 +271,7 @@ static int hd44780_probe(struct platform_device *pdev) return 0; fail: - kfree(lcd); + charlcd_free(lcd); return ret; } @@ -281,7 +281,7 @@ static int hd44780_remove(struct platform_device *pdev) charlcd_unregister(lcd); - kfree(lcd); + charlcd_free(lcd); return 0; } -- 2.20.1
Re: [PATCH 1/3] arm64: dts: fsl: librem5: Add a device tree for the Librem5 devkit
Hi Fabio,
On 2019-03-11 17:10, Fabio Estevam wrote:
On Mon, Mar 11, 2019 at 8:47 PM Angus Ainslie (Purism)
wrote:
+/ {
+ model = "Purism Librem 5 devkit 1.0";
+ compatible = "fsl,librem5-devkit", "fsl,imx8mq";
This board is not manufactured by FSL/NXP, so it should be
"purism,librem5-devkit", "fsl,imx8mq" instead.
You should also add an entry for the purism vendor entry in
Documentation/devicetree/bindings/vendor-prefixes.txt in a separate
patch.
Thanks I'll add it in there.
+
+ chosen {
+ stdout-path =
+ };
+
+ reg_usdhc2_vmmc: regulator-vsd-3v3 {
The usual format would be:
reg_usdhc2_vmmc: regulator-usdhc2-vmmc {
Ok
+ compatible = "regulator-fixed";
+ regulator-name = "VSD_3V3";
+ regulator-min-microvolt = <330>;
+ regulator-max-microvolt = <330>;
+ gpio = < 19 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ regulator-always-on;
Always on? It would be better to pass this regulator inside the mmc
node.
This GPIO is a #disable line for the WLAN and if it goes low the module
doesn't recover. Until we get the WLAN driver working after disable it's
best to leave it always on.
+ };
+
+ reg_pwr_en: pwr_en {
reg_pwr_en: regulator-pwr-en {
Ok
+ compatible = "regulator-fixed";
+ regulator-name = "PWR_EN";
+ regulator-min-microvolt = <330>;
+ regulator-max-microvolt = <330>;
+ gpio = < 8 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ regulator-always-on;
Same here. No need for "regulator-always-on" as it is controlled by
the gyroscope.
This controls a regulator that feeds 80% of the peripherals on the board
and we don't have all of the drivers in the devicetree yet. Shutting
this off would during runtime would break the board so for now it needs
to stay always on.
+ {
+ pinctrl-names = "default";
+ pinctrl-0 = <_fec1>;
+ phy-mode = "rgmii-id";
+ phy-handle = <>;
+ fsl,magic-packet;
+ status = "okay";
+ phy-supply = <_pwr_en>;
+
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ethphy0: ethernet-phy@0 {
+ compatible = "ethernet-phy-ieee802.3-c22";
+ reg = <1>;
You pass @0 and use reg = <1>, which is a mismatch. Building it with
W=1 would have warned you about this mismatch.
Thanks I'll fix that
+ at803x,led-act-blind-workaround;
+ at803x,eee-disabled;
+ power-supply = <_pwr_en>;
+ };
+ };
+};
+
+ {
+ imx8m-som {
No need for this imx8m-som level.
+ pinctrl_nc: ncgrp {
Not a very descriptive naming.
Ok , this was my list of not connected pins but it should be removed.
+ fsl,pins = <
+ MX8MQ_IOMUXC_SAI1_MCLK_SAI1_MCLK0x00
+ MX8MQ_IOMUXC_I2C4_SCL_I2C4_SCL
0x407f
+ MX8MQ_IOMUXC_I2C4_SDA_I2C4_SDA
0x407f
+ >;
+ };
+
+ pinctrl_up: upgrp {
Same here. Also, this is not used. Please remove it.
Ok
+ fsl,pins = <
+ MX8MQ_IOMUXC_SAI1_TXD2_SAI1_TX_DATA2
0x00
+ >;
+ };
+
+ pinctrl_csi1: csi1grp {
This is not used at the moment. Please remove it and re-add it when
CSI gets supported.
Ok
+ fsl,pins = <
+ /* CSI_nRST */
+ MX8MQ_IOMUXC_GPIO1_IO06_GPIO1_IO6
0x11
+ /* CSI_PWDN */
+ MX8MQ_IOMUXC_GPIO1_IO07_GPIO1_IO7
0x19
+ /* CLK01 */
+ MX8MQ_IOMUXC_GPIO1_IO14_CCMSRCGPCMIX_CLKO1
0x19
+ >;
+ };
+
+ pinctrl_pwr_en: pwr_engrp {
+ fsl,pins = <
+ MX8MQ_IOMUXC_GPIO1_IO08_GPIO1_IO8
0x06
+ >;
+ };
+
+ pinctrl_wwan: wwan_grp {
Not used. Please remove this one and all unused pinctrl nodes.
This one should have been used but I'll go through and checked the rest
as well.
+ {
+ clock-frequency = <40>;
+ pinctrl-names = "default";
+ pinctrl-0 = <_i2c1>;
+ status = "okay";
+
+ pmic: bd71837@4b {
Node names should be generic: pmic@4b
+ typec_ptn5100: ptn5110@52 {
Same here.
Ok
+ charger: charger@6b { /* bq25896 */
+ compatible = "ti,bq25890";
+ reg = <0x6b>;
+ pinctrl-names = "default";
+ pinctrl-0 = <_charger>;
+
Re: [PATCH v2 2/3] dt-bindings: mmc: Add a new property disable-cqe-dcmd.
Am Freitag, 8. März 2019, 14:10:45 CET schrieb Christoph Müllner: > > > On 08.03.2019, at 13:46, Adrian Hunter wrote: > > > > On 7/03/19 10:43 AM, Christoph Muellner wrote: > >> This patch documents the new property disable-cqe-dcmd > >> for the Arasan eMMC 5.1 driver. > >> > >> Signed-off-by: Christoph Muellner > >> > >> Signed-off-by: Philipp Tomsich > >> --- > >> Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 4 > >> 1 file changed, 4 insertions(+) > >> > >> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt > >> b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt > >> index 1edbb049cccb..ec699bf98b7c 100644 > >> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt > >> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt > >> @@ -44,6 +44,10 @@ Optional Properties: > >> properly. Test mode can be used to force the controller to function. > >> - xlnx,int-clock-stable-broken: when present, the controller always > >> reports > >> that the internal clock is stable even when it is not. > >> + - disable-cqe-dcmd: The eMMC 5.1 standard specifies direct commands > >> (DCMDs) > >> +as part of the command queue engine (CQE). On controllers with a > >> CQHCI, > >> +such as the Arasan eMMC 5.1 host controller, the driver has to enable > >> DCMDs. > >> +This is done unless disable-cqe-dcmd is specified. This needs a rewording please. See below for hw-description vs. driver, so the description should be centered around why this is a property of the hw [like faulty controller implementation or whatever] > > If "supports-cqe" is in mmc.txt, should "disable-cqe-dcmd" be there also? > > The file mmc.txt says on top: > "These properties are common to multiple MMC host controllers". > As my patchset introduces "disable-cqe-dcmd" just for sdhci-of-arasan, > I would say it should not go into that file. > > Also I wonder why "supports-cqe" is in mmc.txt, because > only sdhci-tegra.c is evaluating that property. > So I would expect it to be documented in > Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt > > However, I see that "disable-cqe-dcmd" could go into other drivers as well. > But is this enough to document it in mmc.txt? Devicetree is always about describing the hardware capabilites and never about the actual nitty-gritty of driver implementation, aka it is not meant as a space for hardware-independent config-settings or such. As for only tegra evaluating this, is probably because it is still so new, like january 2019 and Rob explicitly suggested it becoming common [0], which suggests that the disable-cqe-dcmds should probably also be common. Heiko [0] https://lore.kernel.org/patchwork/patch/1031163/
[PATCH v1 1/4] auxdisplay: hd44780: Fix memory leak on ->remove()
We have to free on ->remove() the allocated resources on ->probe().
Fixes: d47d88361fee ("auxdisplay: Add HD44780 Character LCD support")
Cc: Geert Uytterhoeven
Signed-off-by: Andy Shevchenko
---
drivers/auxdisplay/hd44780.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/auxdisplay/hd44780.c b/drivers/auxdisplay/hd44780.c
index 9ad93ea42fdc..3cde351fb5c9 100644
--- a/drivers/auxdisplay/hd44780.c
+++ b/drivers/auxdisplay/hd44780.c
@@ -280,6 +280,8 @@ static int hd44780_remove(struct platform_device *pdev)
struct charlcd *lcd = platform_get_drvdata(pdev);
charlcd_unregister(lcd);
+
+ kfree(lcd);
return 0;
}
--
2.20.1
Re: [PATCH v2] drivers: uio: Kconfig: pedantic formatting
On Thu, Mar 07, 2019 at 03:22:34AM +0100, Enrico Weigelt, metux IT consult wrote: > Formatting of Kconfig doesn't look so pretty, so just take > a damp cloth and clean it up. > > Signed-off-by: Enrico Weigelt, metux IT consult As before, please fix up the s-o-b line. thanks, greg k-h
Re: [PATCH v9 2/2] pwm: sifive: Add a driver for SiFive SoC PWM
On Tue, Mar 12, 2019 at 01:12:18PM +0100, Thierry Reding wrote:
> On Tue, Mar 12, 2019 at 10:17:39AM +0100, Uwe Kleine-König wrote:
> > Hello,
> >
> > there are just a few minor things left I commented below.
> >
> > On Tue, Mar 12, 2019 at 01:41:29PM +0530, Yash Shah wrote:
> > > +#define div_u64_round(a, b) \
> > > + ({typeof(b) __b = b; div_u64((a) + __b / 2, __b); })
> >
> > Parenthesis around b please. I guess I didn't had them in my suggestion
> > either, sorry for that.
>
> We don't really need the parentheses here, do we? The only operator that
> has lower priority than the assignment is the comma operator, and that's
> not going to work in the macro anyway, unless you put it inside a pair
> of parentheses, in which case, well, you have the parentheses already.
I thought that, too, but using parenthesis just always is a safe bet and
prevents people stumbling over this and spending time to come to the
conclusion that it is actually safe without them.
> > > +static int pwm_sifive_enable(struct pwm_chip *chip, bool enable)
> > > +{
> > > + struct pwm_sifive_ddata *pwm = pwm_sifive_chip_to_ddata(chip);
> > > + int ret;
> > > +
> > > + if (enable) {
> > > + ret = clk_enable(pwm->clk);
> > > + if (ret) {
> > > + dev_err(pwm->chip.dev, "Enable clk failed:%d\n", ret);
> > > + return ret;
> > > + }
> > > + }
> > > +
> > > + if (!enable)
> > > + clk_disable(pwm->clk);
> > > +
> > > + return 0;
> > > +}
> >
> > There is only a single caller for this function. I wonder if it is
> > trivial enough to fold it into pwm_sifive_apply.
>
> I think this is fine. pwm_sifive_apply() is already fairly long at this
> point, so might as well split things up a little.
I don't have a strong opinion here, so keeping as is is fine for me.
> > There are a few other things that could be improved, but I think they
> > could be addressed later. For some of these I don't even know what to
> > suggest, for some Thierry might not agree it is worth fixing:
> >
> > - rounding
> >how to round? When should a request declined, when is rounding ok?
> >There is still "if (state->period != pwm->approx_period) return -EBUSY"
> >in this driver. This is better than before, but if state-period ==
> >pwm->approx_period + 1 the result (if accepted) might be the same as
> >without the +1 and so returning -EBUSY for one case and accepting the
> >other is strange.
>
> Perhaps a good idea would be to reject a configuration only after we've
> determined that it is incompatible? If we're really going to end up with
> the same configuration within a given margin of period or duty cycle and
> we can't do much about it, there's little point in rejecting such
> configurations.
It seems we agree here. Is this important enough to delay taking this
driver further? Currently the driver rejects too broad so if it annoys
someone this can still be fixed later and there is only little harm
(assuming correct error handling in the consumers).
> > - don't call PWM API functions designed for consumers (here: pwm_get_state)
>
> Agreed. The driver can just access pwm_device.state directly.
I wouldn't do this either. IMHO the driver should only look into its
hardware registers instead of using framework interna (or consumer API
calls).
> > - Move div_u64_round to include/linux/math64.h
>
> Looks to me like DIV_ROUND_CLOSEST_ULL() is already pretty much this.
> The only difference that I can see is that the divisor is 32-bit, but
> since we pass in state->period, that already works fine.
num (i.e. the divident) should be a u64, but it isn't. This needs
fixing. But I agree that DIV_ROUND_CLOSEST_ULL should be good enough
then.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/ |
