Re: [PATCH v3 4/8] gpio: dt-bindings: Add documentation for Aspeed GPIO controllers

[Rob Herring]

On Tue, Aug 30, 2016 at 05:24:23PM +0930, Andrew Jeffery wrote:
> Signed-off-by: Andrew Jeffery 
> Acked-by: Joel Stanley 
> ---
>  .../devicetree/bindings/gpio/gpio-aspeed.txt   | 36 
> ++
>  1 file changed, 36 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-aspeed.txt

Acked-by: Rob Herring 

> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-aspeed.txt 
> b/Documentation/devicetree/bindings/gpio/gpio-aspeed.txt
> new file mode 100644
> index ..09f26cac18d1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-aspeed.txt
> @@ -0,0 +1,36 @@
> +Aspeed GPIO controller Device Tree Bindings
> +---
> +
> +Required properties:
> +- #gpio-cells: Should be two
> +   - First cell is the GPIO line number
> +   - Second cell is used to specify optional
> + parameters (unused)
> +
> +- compatible : Either "aspeed,ast2400-gpio" or "aspeed,ast2500-gpio"

I'd prefer this be listed first if you respin this.

> +
> +- reg: Address and length of the register set for 
> the device
> +- gpio-controller: Marks the device node as a GPIO controller.
> +- interrupts : Interrupt specifier (see interrupt bindings for
> +   details)
> +- interrupt-controller   : Mark the GPIO controller as an 
> interrupt-controller
> +
> +Optional properties:
> +
> +- interrupt-parent : The parent interrupt controller, optional if 
> inherited
> +
> +The gpio and interrupt properties are further described in their respective
> +bindings documentation:
> +
> +- Documentation/devicetree/bindings/gpio/gpio.txt
> +- Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> +
> +  Example:
> + gpio@1e78 {
> + #gpio-cells = <2>;
> + compatible = "aspeed,ast2400-gpio";
> + gpio-controller;
> + interrupts = <20>;
> + reg = <0x1e78 0x1000>;
> + interrupt-controller;
> + };
> -- 
> 2.9.3.1.g0db844e
> 

Re: [PATCH v3 4/8] gpio: dt-bindings: Add documentation for Aspeed GPIO controllers

[Rob Herring]

On Tue, Aug 30, 2016 at 05:24:23PM +0930, Andrew Jeffery wrote:
> Signed-off-by: Andrew Jeffery 
> Acked-by: Joel Stanley 
> ---
>  .../devicetree/bindings/gpio/gpio-aspeed.txt   | 36 
> ++
>  1 file changed, 36 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-aspeed.txt

Acked-by: Rob Herring 

> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-aspeed.txt 
> b/Documentation/devicetree/bindings/gpio/gpio-aspeed.txt
> new file mode 100644
> index ..09f26cac18d1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-aspeed.txt
> @@ -0,0 +1,36 @@
> +Aspeed GPIO controller Device Tree Bindings
> +---
> +
> +Required properties:
> +- #gpio-cells: Should be two
> +   - First cell is the GPIO line number
> +   - Second cell is used to specify optional
> + parameters (unused)
> +
> +- compatible : Either "aspeed,ast2400-gpio" or "aspeed,ast2500-gpio"

I'd prefer this be listed first if you respin this.

> +
> +- reg: Address and length of the register set for 
> the device
> +- gpio-controller: Marks the device node as a GPIO controller.
> +- interrupts : Interrupt specifier (see interrupt bindings for
> +   details)
> +- interrupt-controller   : Mark the GPIO controller as an 
> interrupt-controller
> +
> +Optional properties:
> +
> +- interrupt-parent : The parent interrupt controller, optional if 
> inherited
> +
> +The gpio and interrupt properties are further described in their respective
> +bindings documentation:
> +
> +- Documentation/devicetree/bindings/gpio/gpio.txt
> +- Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> +
> +  Example:
> + gpio@1e78 {
> + #gpio-cells = <2>;
> + compatible = "aspeed,ast2400-gpio";
> + gpio-controller;
> + interrupts = <20>;
> + reg = <0x1e78 0x1000>;
> + interrupt-controller;
> + };
> -- 
> 2.9.3.1.g0db844e
> 

Re: RFC: Backport HID-logitech to 3.10?

[Simon Wood]

On Fri, September 2, 2016 3:32 am, Benjamin Tissoires wrote:
> On Thu, Sep 1, 2016 at 2:56 AM, Simon Wood  wrote:
>> After copying the HEAD 'hid-lg.[ch]' and 'hid-lg4ff.[ch]' from 4.7,
>> there is a minimal patch (example attached) required to get the build
>> working.
>
> Don't you need hid-logitech-hidpp too?

Hi Benjamin,
Nope, the 'hid-logitech-hidpp.ko' driver is separate and (for wheels) only
has support for the G920 wheel. Other Logitech wheels are handled by
'hid-logitech.ko'.

> Not sure I understand exactly what you want from us here.

A puppy. ;-)

> The stable rules are detailed in
> Documentation/stable_kernel_rules.txt. And unfortunately, I don't
> think adding these patches will be acceptable for upstream 3.10.

After reading through these I would agree with you, that this is a
nice-to-have not a need-to-have. So, the patch is here if anyone wants to
use it personally or for a distribution.

Thank you for your comments,
Simon.

Re: RFC: Backport HID-logitech to 3.10?

[Simon Wood]

On Fri, September 2, 2016 3:32 am, Benjamin Tissoires wrote:
> On Thu, Sep 1, 2016 at 2:56 AM, Simon Wood  wrote:
>> After copying the HEAD 'hid-lg.[ch]' and 'hid-lg4ff.[ch]' from 4.7,
>> there is a minimal patch (example attached) required to get the build
>> working.
>
> Don't you need hid-logitech-hidpp too?

Hi Benjamin,
Nope, the 'hid-logitech-hidpp.ko' driver is separate and (for wheels) only
has support for the G920 wheel. Other Logitech wheels are handled by
'hid-logitech.ko'.

> Not sure I understand exactly what you want from us here.

A puppy. ;-)

> The stable rules are detailed in
> Documentation/stable_kernel_rules.txt. And unfortunately, I don't
> think adding these patches will be acceptable for upstream 3.10.

After reading through these I would agree with you, that this is a
nice-to-have not a need-to-have. So, the patch is here if anyone wants to
use it personally or for a distribution.

Thank you for your comments,
Simon.

Re: [PATCH v4 2/4] drm: Add API for capturing frame CRCs

[Emil Velikov]

Hi Tomeu,

On 5 August 2016 at 11:45, Tomeu Vizoso  wrote:

> +#ifdef CONFIG_DEBUG_FS
> +   spin_lock_init(>crc.lock);
> +   init_waitqueue_head(>crc.wq);
> +   crtc->crc.source = kstrdup("auto", GFP_KERNEL);
Pedantic: kstrdup() can never fail ?

> +#endif
> +
> if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
> drm_object_attach_property(>base, config->prop_active, 
> 0);
> drm_object_attach_property(>base, config->prop_mode_id, 
> 0);
> @@ -764,6 +771,11 @@ void drm_crtc_cleanup(struct drm_crtc *crtc)
>  * the indices on the drm_crtc after us in the crtc_list.
>  */
>
> +#ifdef CONFIG_DEBUG_FS
> +   drm_debugfs_crtc_remove(crtc);
> +   kfree(crtc->crc.source);
> +#endif
> +
Worth adding these two as static inlines in the header ?

Things are a bit asymetrical here. drm_debugfs_crtc_add() is in
drm_minor_register(), so why isn't drm_debugfs_crtc_remove() in
drm_minor_free() ?



> -#endif /* CONFIG_DEBUG_FS */
> +int drm_debugfs_crtc_add(struct drm_crtc *crtc)
> +{
> +   struct drm_minor *minor = crtc->dev->primary;
> +   struct dentry *root;
> +   char *name;
> +
> +   name = kasprintf(GFP_KERNEL, "crtc-%d", crtc->index);
Mildly related: some userspace projects define convenience helpers
which you use in order to allocate the correct amount of space on
stack.
Is there such a set for the kernel ?

With those the above will become something like:
char name[5+DECIMAL_STR_MAX] = ksprintf("crtc-%d", crtc->index);



> --- /dev/null
> +++ b/drivers/gpu/drm/drm_debugfs_crc.c

> + *
> + * Authors:
> + *Eric Anholt 
> + *Keith Packard 
> + *
Wondering about these authorship lines - is the code copied/derived
from somewhere ? If so please mention where from.

> +/*
> + * 1 frame field of 10 chars plus a number of CRC fields of 10 chars each, 
> space
> + * separated and with a newline at the end.
> + */
> +#define LINE_LEN(VALUES_CNT)   (10 + 11 * VALUES_CNT + 1 + 1)
Hmm I seem to suck at math. The macro seems larger than expected (comment and
code wise) by 1, right ?

> +#define MAX_LINE_LEN   (LINE_LEN(DRM_MAX_CRC_NR))
> +
Worth using lowercase VALUES_CNT and less likely to clash macro names ?



> +   BUILD_BUG_ON_NOT_POWER_OF_2(DRM_CRC_ENTRIES_NR);
> +   crc->tail = (crc->tail + 1) & (DRM_CRC_ENTRIES_NR - 1);
It's strange that the kernel does not have a macro for this. But for the sake
of me I cannot find one.



> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -192,6 +192,7 @@ static void drm_minor_free(struct drm_device *dev, 
> unsigned int type)
>  static int drm_minor_register(struct drm_device *dev, unsigned int type)
>  {
> struct drm_minor *minor;
> +   struct drm_crtc *crtc;
> unsigned long flags;
> int ret;
>
> @@ -207,6 +208,14 @@ static int drm_minor_register(struct drm_device *dev, 
> unsigned int type)
> return ret;
> }
>
> +   if (type == DRM_MINOR_LEGACY) {
Sidenote: If we did not use DRM_MINOR_LEGACY for render-only GPUs we would
save a couple of cycles here :-)

> +   drm_for_each_crtc(crtc, dev) {
> +   ret = drm_debugfs_crtc_add(crtc);
> +   if (ret)
> +   goto err_debugfs;
IMHO we don't want to bring down the whole setup if this fails, unlike
where we cannot create debug/dri all together.

Regardless: we seems to be missing the cleanup in the error path ?



> +#if defined(CONFIG_DEBUG_FS)
> +extern const struct file_operations drm_crc_control_fops;
> +extern const struct file_operations drm_crtc_crc_fops;
These seems to be named differently in the code. Namely:

drm_crtc_crc_control_fops
drm_crtc_crc_data_fops

Then again, you don't seem to use the outside of the file, so I'd make them
static and drop this hunk.



>  * before data structures are torndown.
>  */
> void (*early_unregister)(struct drm_crtc *crtc);
> +
> +   /**
> +* @set_crc_source:
> +*
> +* Changes the source of CRC checksums of frames at the request of
> +* userspace, typically for testing purposes. The sources available 
> are
> +* specific of each driver and a %NULL value indicates that CRC
> +* generation is to be switched off.
> +*
> +* When CRC generation is enabled, the driver should call
> +* drm_crtc_add_crc_entry() at each frame, providing any information
> +* that characterizes the frame contents in the crcN arguments, as
> +* provided from the configured source. Drivers should accept a "auto"
Very nicely written documentation. Just to make it more obvious - s/should/must/


> +/**
> + * struct drm_crtc_crc_entry - entry describing a frame's content
> + * @frame: number of the frame this CRC is about
> + * @crc: array of values that characterize the frame
> + */

Re: [PATCH v4 2/4] drm: Add API for capturing frame CRCs

[Emil Velikov]

Hi Tomeu,

On 5 August 2016 at 11:45, Tomeu Vizoso  wrote:

> +#ifdef CONFIG_DEBUG_FS
> +   spin_lock_init(>crc.lock);
> +   init_waitqueue_head(>crc.wq);
> +   crtc->crc.source = kstrdup("auto", GFP_KERNEL);
Pedantic: kstrdup() can never fail ?

> +#endif
> +
> if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
> drm_object_attach_property(>base, config->prop_active, 
> 0);
> drm_object_attach_property(>base, config->prop_mode_id, 
> 0);
> @@ -764,6 +771,11 @@ void drm_crtc_cleanup(struct drm_crtc *crtc)
>  * the indices on the drm_crtc after us in the crtc_list.
>  */
>
> +#ifdef CONFIG_DEBUG_FS
> +   drm_debugfs_crtc_remove(crtc);
> +   kfree(crtc->crc.source);
> +#endif
> +
Worth adding these two as static inlines in the header ?

Things are a bit asymetrical here. drm_debugfs_crtc_add() is in
drm_minor_register(), so why isn't drm_debugfs_crtc_remove() in
drm_minor_free() ?



> -#endif /* CONFIG_DEBUG_FS */
> +int drm_debugfs_crtc_add(struct drm_crtc *crtc)
> +{
> +   struct drm_minor *minor = crtc->dev->primary;
> +   struct dentry *root;
> +   char *name;
> +
> +   name = kasprintf(GFP_KERNEL, "crtc-%d", crtc->index);
Mildly related: some userspace projects define convenience helpers
which you use in order to allocate the correct amount of space on
stack.
Is there such a set for the kernel ?

With those the above will become something like:
char name[5+DECIMAL_STR_MAX] = ksprintf("crtc-%d", crtc->index);



> --- /dev/null
> +++ b/drivers/gpu/drm/drm_debugfs_crc.c

> + *
> + * Authors:
> + *Eric Anholt 
> + *Keith Packard 
> + *
Wondering about these authorship lines - is the code copied/derived
from somewhere ? If so please mention where from.

> +/*
> + * 1 frame field of 10 chars plus a number of CRC fields of 10 chars each, 
> space
> + * separated and with a newline at the end.
> + */
> +#define LINE_LEN(VALUES_CNT)   (10 + 11 * VALUES_CNT + 1 + 1)
Hmm I seem to suck at math. The macro seems larger than expected (comment and
code wise) by 1, right ?

> +#define MAX_LINE_LEN   (LINE_LEN(DRM_MAX_CRC_NR))
> +
Worth using lowercase VALUES_CNT and less likely to clash macro names ?



> +   BUILD_BUG_ON_NOT_POWER_OF_2(DRM_CRC_ENTRIES_NR);
> +   crc->tail = (crc->tail + 1) & (DRM_CRC_ENTRIES_NR - 1);
It's strange that the kernel does not have a macro for this. But for the sake
of me I cannot find one.



> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -192,6 +192,7 @@ static void drm_minor_free(struct drm_device *dev, 
> unsigned int type)
>  static int drm_minor_register(struct drm_device *dev, unsigned int type)
>  {
> struct drm_minor *minor;
> +   struct drm_crtc *crtc;
> unsigned long flags;
> int ret;
>
> @@ -207,6 +208,14 @@ static int drm_minor_register(struct drm_device *dev, 
> unsigned int type)
> return ret;
> }
>
> +   if (type == DRM_MINOR_LEGACY) {
Sidenote: If we did not use DRM_MINOR_LEGACY for render-only GPUs we would
save a couple of cycles here :-)

> +   drm_for_each_crtc(crtc, dev) {
> +   ret = drm_debugfs_crtc_add(crtc);
> +   if (ret)
> +   goto err_debugfs;
IMHO we don't want to bring down the whole setup if this fails, unlike
where we cannot create debug/dri all together.

Regardless: we seems to be missing the cleanup in the error path ?



> +#if defined(CONFIG_DEBUG_FS)
> +extern const struct file_operations drm_crc_control_fops;
> +extern const struct file_operations drm_crtc_crc_fops;
These seems to be named differently in the code. Namely:

drm_crtc_crc_control_fops
drm_crtc_crc_data_fops

Then again, you don't seem to use the outside of the file, so I'd make them
static and drop this hunk.



>  * before data structures are torndown.
>  */
> void (*early_unregister)(struct drm_crtc *crtc);
> +
> +   /**
> +* @set_crc_source:
> +*
> +* Changes the source of CRC checksums of frames at the request of
> +* userspace, typically for testing purposes. The sources available 
> are
> +* specific of each driver and a %NULL value indicates that CRC
> +* generation is to be switched off.
> +*
> +* When CRC generation is enabled, the driver should call
> +* drm_crtc_add_crc_entry() at each frame, providing any information
> +* that characterizes the frame contents in the crcN arguments, as
> +* provided from the configured source. Drivers should accept a "auto"
Very nicely written documentation. Just to make it more obvious - s/should/must/


> +/**
> + * struct drm_crtc_crc_entry - entry describing a frame's content
> + * @frame: number of the frame this CRC is about
> + * @crc: array of values that characterize the frame
> + */
> +struct drm_crtc_crc_entry {
> +   bool 

Re: [writeback] 8bc4ad9498: INFO: suspicious RCU usage. ]

[Tejun Heo]

(cc'ing Paul, hi!)

Hello,

On Thu, Sep 01, 2016 at 02:13:34PM -0600, Jens Axboe wrote:
> On 09/01/2016 04:21 AM, kernel test robot wrote:
> > [7.323356] cdrom: Uniform CD-ROM driver Revision: 3.20
> > [7.334239]
> > [7.337256] ===
> > [7.340532] [ INFO: suspicious RCU usage. ]
> > [7.342419] 4.8.0-rc4-8-g8bc4ad9 #1 Not tainted
> > [7.347065] ---
> > [7.350132] include/linux/cgroup.h:435 suspicious 
> > rcu_dereference_check() usage!
...
> > [7.410074] Call Trace:
> > [7.411328]  [] dump_stack+0x82/0xb8
> > [7.413982]  [] lockdep_rcu_suspicious+0xf7/0x100
> > [7.415828]  [] bio_blkcg+0x89/0x93
> > [7.417336]  [] check_blkcg_changed+0x58/0x1b8
> > [7.428722]  [] cfq_set_request+0xd1/0x2a3
> > [7.439690]  [] elv_set_request+0x1f/0x24
> > [7.442157]  [] get_request+0x38f/0x77f
> > [7.447449]  [] blk_get_request+0x65/0xa8
> > [7.449868]  [] ide_cd_queue_pc+0x76/0x19d
> > [7.453757]  [] cdrom_check_status+0x51/0x53
> > [7.455372]  [] ide_cdrom_check_events_real+0x20/0x3f
> > [7.457294]  [] cdrom_update_events+0x18/0x21
> > [7.458987]  [] cdrom_check_events+0x12/0x1f
> > [7.460713]  [] idecd_check_events+0x1c/0x1e
> > [7.462393]  [] disk_check_events+0x47/0x103
> > [7.464129]  [] disk_events_workfn+0x1c/0x1e
> > [7.465844]  [] process_one_work+0x272/0x4ee
> > [7.467462]  [] worker_thread+0x1eb/0x2c9

The warning is from

#define task_css_set_check(task, __c)   \
rcu_dereference_check((task)->cgroups,  \
lockdep_is_held(_mutex) ||   \
lockdep_is_held(_set_lock) ||   \
((task)->flags & PF_EXITING) || (__c))

which is used by bio_blkcg() which is called by the following code in
check_blkcg_changed().

rcu_read_lock();
serial_nr = bio_blkcg(bio)->css.serial_nr;
rcu_read_unlock();

So, I have no idea.  It looks like rcu_dereference_check() is being
called with rcu read locked but still triggering suspicious RCU usage
warning.

The code hasn't changed for quite a while now, so it's also really
weird that it's triggering now.  Paul, does anything ring a bell?

Thanks.

-- 
tejun

Re: [writeback] 8bc4ad9498: INFO: suspicious RCU usage. ]

[Tejun Heo]

(cc'ing Paul, hi!)

Hello,

On Thu, Sep 01, 2016 at 02:13:34PM -0600, Jens Axboe wrote:
> On 09/01/2016 04:21 AM, kernel test robot wrote:
> > [7.323356] cdrom: Uniform CD-ROM driver Revision: 3.20
> > [7.334239]
> > [7.337256] ===
> > [7.340532] [ INFO: suspicious RCU usage. ]
> > [7.342419] 4.8.0-rc4-8-g8bc4ad9 #1 Not tainted
> > [7.347065] ---
> > [7.350132] include/linux/cgroup.h:435 suspicious 
> > rcu_dereference_check() usage!
...
> > [7.410074] Call Trace:
> > [7.411328]  [] dump_stack+0x82/0xb8
> > [7.413982]  [] lockdep_rcu_suspicious+0xf7/0x100
> > [7.415828]  [] bio_blkcg+0x89/0x93
> > [7.417336]  [] check_blkcg_changed+0x58/0x1b8
> > [7.428722]  [] cfq_set_request+0xd1/0x2a3
> > [7.439690]  [] elv_set_request+0x1f/0x24
> > [7.442157]  [] get_request+0x38f/0x77f
> > [7.447449]  [] blk_get_request+0x65/0xa8
> > [7.449868]  [] ide_cd_queue_pc+0x76/0x19d
> > [7.453757]  [] cdrom_check_status+0x51/0x53
> > [7.455372]  [] ide_cdrom_check_events_real+0x20/0x3f
> > [7.457294]  [] cdrom_update_events+0x18/0x21
> > [7.458987]  [] cdrom_check_events+0x12/0x1f
> > [7.460713]  [] idecd_check_events+0x1c/0x1e
> > [7.462393]  [] disk_check_events+0x47/0x103
> > [7.464129]  [] disk_events_workfn+0x1c/0x1e
> > [7.465844]  [] process_one_work+0x272/0x4ee
> > [7.467462]  [] worker_thread+0x1eb/0x2c9

The warning is from

#define task_css_set_check(task, __c)   \
rcu_dereference_check((task)->cgroups,  \
lockdep_is_held(_mutex) ||   \
lockdep_is_held(_set_lock) ||   \
((task)->flags & PF_EXITING) || (__c))

which is used by bio_blkcg() which is called by the following code in
check_blkcg_changed().

rcu_read_lock();
serial_nr = bio_blkcg(bio)->css.serial_nr;
rcu_read_unlock();

So, I have no idea.  It looks like rcu_dereference_check() is being
called with rcu read locked but still triggering suspicious RCU usage
warning.

The code hasn't changed for quite a while now, so it's also really
weird that it's triggering now.  Paul, does anything ring a bell?

Thanks.

-- 
tejun

Re: [PATCH net-next 3/3] net: dsa: mv88e6xxx: make global2 code optional

[Andrew Lunn]

On Fri, Sep 02, 2016 at 08:08:19AM -0400, Vivien Didelot wrote:
> Since not every chip has a Global2 set of registers, make its support
> optional, in which case the related functions will return -EOPNOTSUPP.
> 
> This also allows to reduce the size of the mv88e6xxx driver for devices
> such as home routers embedding Ethernet chips without Global2 support.

Hi Vivien

I've no problems with splitting this out.

However, making it optional? I don't think that is a good idea.

1) If you don't have this, and you should, it looks like the PHYs stop
working, and since you cannot set the switch MAC address, maybe Pause
frames are broken.

2) Distros won't build a kernel per target hardware. They probably
build a kernel per SoC vendor. That means, they will have this
optional code built all the time. Very few builds will leave it out.

So the only way i think making this optional, is if it is a kernel
module, and it gets loaded if needed.

Andrew

Re: [PATCH net-next 3/3] net: dsa: mv88e6xxx: make global2 code optional

[Andrew Lunn]

On Fri, Sep 02, 2016 at 08:08:19AM -0400, Vivien Didelot wrote:
> Since not every chip has a Global2 set of registers, make its support
> optional, in which case the related functions will return -EOPNOTSUPP.
> 
> This also allows to reduce the size of the mv88e6xxx driver for devices
> such as home routers embedding Ethernet chips without Global2 support.

Hi Vivien

I've no problems with splitting this out.

However, making it optional? I don't think that is a good idea.

1) If you don't have this, and you should, it looks like the PHYs stop
working, and since you cannot set the switch MAC address, maybe Pause
frames are broken.

2) Distros won't build a kernel per target hardware. They probably
build a kernel per SoC vendor. That means, they will have this
optional code built all the time. Very few builds will leave it out.

So the only way i think making this optional, is if it is a kernel
module, and it gets loaded if needed.

Andrew

[PATCH] serial: earlycon: Extend earlycon command line option to support 64-bit addresses

[Alexander Sverdlin]

earlycon implementation used "unsigned long" internally, but there are systems
(ARM with LPAE) where sizeof(unsigned long) == 4 and uart is mapped beyond 4GiB
address range.

Switch to resource_size_t internally and replace obsoleted simple_strtoul() with
kstrtoull().

Signed-off-by: Alexander Sverdlin 
---
 drivers/tty/serial/8250/8250_core.c |  2 +-
 drivers/tty/serial/earlycon.c   |  7 +++
 drivers/tty/serial/serial_core.c| 12 +---
 include/linux/serial_core.h |  2 +-
 4 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c 
b/drivers/tty/serial/8250/8250_core.c
index 13ad5c3..f64d6cd 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -639,7 +639,7 @@ static int univ8250_console_match(struct console *co, char 
*name, int idx,
 {
char match[] = "uart";  /* 8250-specific earlycon name */
unsigned char iotype;
-   unsigned long addr;
+   resource_size_t addr;
int i;
 
if (strncmp(name, match, 4) != 0)
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index 067783f..3940280 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -38,7 +38,7 @@ static struct earlycon_device early_console_dev = {
.con = _con,
 };
 
-static void __iomem * __init earlycon_map(unsigned long paddr, size_t size)
+static void __iomem * __init earlycon_map(resource_size_t paddr, size_t size)
 {
void __iomem *base;
 #ifdef CONFIG_FIX_EARLYCON_MEM
@@ -49,8 +49,7 @@ static void __iomem * __init earlycon_map(unsigned long 
paddr, size_t size)
base = ioremap(paddr, size);
 #endif
if (!base)
-   pr_err("%s: Couldn't map 0x%llx\n", __func__,
-  (unsigned long long)paddr);
+   pr_err("%s: Couldn't map %pa\n", __func__, );
 
return base;
 }
@@ -92,7 +91,7 @@ static int __init parse_options(struct earlycon_device 
*device, char *options)
 {
struct uart_port *port = >port;
int length;
-   unsigned long addr;
+   resource_size_t addr;
 
if (uart_parse_earlycon(options, >iotype, , ))
return -EINVAL;
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 9fc1533..89f5d6a 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1938,11 +1938,14 @@ uart_get_console(struct uart_port *ports, int nr, 
struct console *co)
  *console=,0x,
  * is also accepted; the returned @iotype will be UPIO_MEM.
  *
- * Returns 0 on success or -EINVAL on failure
+ * Returns 0 on success, -EINVAL or -ERANGE on failure
  */
-int uart_parse_earlycon(char *p, unsigned char *iotype, unsigned long *addr,
+int uart_parse_earlycon(char *p, unsigned char *iotype, resource_size_t *addr,
char **options)
 {
+   int ret;
+   unsigned long long tmp;
+
if (strncmp(p, "mmio,", 5) == 0) {
*iotype = UPIO_MEM;
p += 5;
@@ -1968,7 +1971,10 @@ int uart_parse_earlycon(char *p, unsigned char *iotype, 
unsigned long *addr,
return -EINVAL;
}
 
-   *addr = simple_strtoul(p, NULL, 0);
+   ret = kstrtoull(p, 0, );
+   if (ret)
+   return ret;
+   *addr = tmp;
p = strchr(p, ',');
if (p)
p++;
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 2f44e20..cdba6f1 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -374,7 +374,7 @@ extern int of_setup_earlycon(const struct earlycon_id 
*match,
 
 struct uart_port *uart_get_console(struct uart_port *ports, int nr,
   struct console *c);
-int uart_parse_earlycon(char *p, unsigned char *iotype, unsigned long *addr,
+int uart_parse_earlycon(char *p, unsigned char *iotype, resource_size_t *addr,
char **options);
 void uart_parse_options(char *options, int *baud, int *parity, int *bits,
int *flow);

[PATCH] serial: earlycon: Extend earlycon command line option to support 64-bit addresses

[Alexander Sverdlin]

earlycon implementation used "unsigned long" internally, but there are systems
(ARM with LPAE) where sizeof(unsigned long) == 4 and uart is mapped beyond 4GiB
address range.

Switch to resource_size_t internally and replace obsoleted simple_strtoul() with
kstrtoull().

Signed-off-by: Alexander Sverdlin 
---
 drivers/tty/serial/8250/8250_core.c |  2 +-
 drivers/tty/serial/earlycon.c   |  7 +++
 drivers/tty/serial/serial_core.c| 12 +---
 include/linux/serial_core.h |  2 +-
 4 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c 
b/drivers/tty/serial/8250/8250_core.c
index 13ad5c3..f64d6cd 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -639,7 +639,7 @@ static int univ8250_console_match(struct console *co, char 
*name, int idx,
 {
char match[] = "uart";  /* 8250-specific earlycon name */
unsigned char iotype;
-   unsigned long addr;
+   resource_size_t addr;
int i;
 
if (strncmp(name, match, 4) != 0)
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index 067783f..3940280 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -38,7 +38,7 @@ static struct earlycon_device early_console_dev = {
.con = _con,
 };
 
-static void __iomem * __init earlycon_map(unsigned long paddr, size_t size)
+static void __iomem * __init earlycon_map(resource_size_t paddr, size_t size)
 {
void __iomem *base;
 #ifdef CONFIG_FIX_EARLYCON_MEM
@@ -49,8 +49,7 @@ static void __iomem * __init earlycon_map(unsigned long 
paddr, size_t size)
base = ioremap(paddr, size);
 #endif
if (!base)
-   pr_err("%s: Couldn't map 0x%llx\n", __func__,
-  (unsigned long long)paddr);
+   pr_err("%s: Couldn't map %pa\n", __func__, );
 
return base;
 }
@@ -92,7 +91,7 @@ static int __init parse_options(struct earlycon_device 
*device, char *options)
 {
struct uart_port *port = >port;
int length;
-   unsigned long addr;
+   resource_size_t addr;
 
if (uart_parse_earlycon(options, >iotype, , ))
return -EINVAL;
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 9fc1533..89f5d6a 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1938,11 +1938,14 @@ uart_get_console(struct uart_port *ports, int nr, 
struct console *co)
  *console=,0x,
  * is also accepted; the returned @iotype will be UPIO_MEM.
  *
- * Returns 0 on success or -EINVAL on failure
+ * Returns 0 on success, -EINVAL or -ERANGE on failure
  */
-int uart_parse_earlycon(char *p, unsigned char *iotype, unsigned long *addr,
+int uart_parse_earlycon(char *p, unsigned char *iotype, resource_size_t *addr,
char **options)
 {
+   int ret;
+   unsigned long long tmp;
+
if (strncmp(p, "mmio,", 5) == 0) {
*iotype = UPIO_MEM;
p += 5;
@@ -1968,7 +1971,10 @@ int uart_parse_earlycon(char *p, unsigned char *iotype, 
unsigned long *addr,
return -EINVAL;
}
 
-   *addr = simple_strtoul(p, NULL, 0);
+   ret = kstrtoull(p, 0, );
+   if (ret)
+   return ret;
+   *addr = tmp;
p = strchr(p, ',');
if (p)
p++;
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 2f44e20..cdba6f1 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -374,7 +374,7 @@ extern int of_setup_earlycon(const struct earlycon_id 
*match,
 
 struct uart_port *uart_get_console(struct uart_port *ports, int nr,
   struct console *c);
-int uart_parse_earlycon(char *p, unsigned char *iotype, unsigned long *addr,
+int uart_parse_earlycon(char *p, unsigned char *iotype, resource_size_t *addr,
char **options);
 void uart_parse_options(char *options, int *baud, int *parity, int *bits,
int *flow);

Re: [git pull] drm fixes for rc5

[Linus Torvalds]

On Thu, Sep 1, 2016 at 10:59 PM, Dave Airlie  wrote:
>
> I've tried using a signed tag, let's see if works.

Worked fine.

But your email was once again marked as spam.

Google hates you, and your email habits.

Linus

Re: [git pull] drm fixes for rc5

[Linus Torvalds]

On Thu, Sep 1, 2016 at 10:59 PM, Dave Airlie  wrote:
>
> I've tried using a signed tag, let's see if works.

Worked fine.

But your email was once again marked as spam.

Google hates you, and your email habits.

Linus

Re: [PATCH 2/5] irqtime: Remove needless IRQs disablement on kcpustat update

[Paolo Bonzini]

On 02/09/2016 16:03, Frederic Weisbecker wrote:
> The callers of the functions performing irqtime kcpustat updates have
> IRQS disabled, no need to disable them again.

They do, but perhaps this should be annotated through some sparse magic.
 It's starting to be hairy, with the requirement spanning many separate
files.

Something like

#define __irq_disabled __must_hold(IRQ)

together with __acquire and __release annotations in
include/linux/irqflags.h would do.  I'm not sure how to handle
local_irq_save/local_irq_restore, but I guess sparse would be fine with

((void)({
   raw_local_irq_save(flags);
   if (flags) __acquire(IRQ);
}))

and

((void)({
   if (flags) __release(IRQ);
   raw_local_irq_restore(flags);
}))

since below that it's assembly.

Starting from irqtime_account_hi_update, irqtime_account_si_update and
irqtime_account_irq you'd get quite a few functions annotated.

Paolo

> Cc: Rik van Riel 
> Cc: Paolo Bonzini 
> Cc: Wanpeng Li 
> Cc: Mike Galbraith 
> Cc: Peter Zijlstra 
> Cc: Ingo Molnar 
> Cc: Eric Dumazet 
> Signed-off-by: Frederic Weisbecker 
> ---
>  kernel/sched/cputime.c | 8 ++--
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
> index f111076..d4d12a9 100644
> --- a/kernel/sched/cputime.c
> +++ b/kernel/sched/cputime.c
> @@ -78,30 +78,26 @@ EXPORT_SYMBOL_GPL(irqtime_account_irq);
>  static cputime_t irqtime_account_hi_update(cputime_t maxtime)
>  {
>   u64 *cpustat = kcpustat_this_cpu->cpustat;
> - unsigned long flags;
>   cputime_t irq_cputime;
>  
> - local_irq_save(flags);
>   irq_cputime = nsecs_to_cputime64(__this_cpu_read(cpu_hardirq_time)) -
> cpustat[CPUTIME_IRQ];
>   irq_cputime = min(irq_cputime, maxtime);
>   cpustat[CPUTIME_IRQ] += irq_cputime;
> - local_irq_restore(flags);
> +
>   return irq_cputime;
>  }
>  
>  static cputime_t irqtime_account_si_update(cputime_t maxtime)
>  {
>   u64 *cpustat = kcpustat_this_cpu->cpustat;
> - unsigned long flags;
>   cputime_t softirq_cputime;
>  
> - local_irq_save(flags);
>   softirq_cputime = nsecs_to_cputime64(__this_cpu_read(cpu_softirq_time)) 
> -
> cpustat[CPUTIME_SOFTIRQ];
>   softirq_cputime = min(softirq_cputime, maxtime);
>   cpustat[CPUTIME_SOFTIRQ] += softirq_cputime;
> - local_irq_restore(flags);
> +
>   return softirq_cputime;
>  }
>  
> 

Re: [PATCH 2/5] irqtime: Remove needless IRQs disablement on kcpustat update

[Paolo Bonzini]

On 02/09/2016 16:03, Frederic Weisbecker wrote:
> The callers of the functions performing irqtime kcpustat updates have
> IRQS disabled, no need to disable them again.

They do, but perhaps this should be annotated through some sparse magic.
 It's starting to be hairy, with the requirement spanning many separate
files.

Something like

#define __irq_disabled __must_hold(IRQ)

together with __acquire and __release annotations in
include/linux/irqflags.h would do.  I'm not sure how to handle
local_irq_save/local_irq_restore, but I guess sparse would be fine with

((void)({
   raw_local_irq_save(flags);
   if (flags) __acquire(IRQ);
}))

and

((void)({
   if (flags) __release(IRQ);
   raw_local_irq_restore(flags);
}))

since below that it's assembly.

Starting from irqtime_account_hi_update, irqtime_account_si_update and
irqtime_account_irq you'd get quite a few functions annotated.

Paolo

> Cc: Rik van Riel 
> Cc: Paolo Bonzini 
> Cc: Wanpeng Li 
> Cc: Mike Galbraith 
> Cc: Peter Zijlstra 
> Cc: Ingo Molnar 
> Cc: Eric Dumazet 
> Signed-off-by: Frederic Weisbecker 
> ---
>  kernel/sched/cputime.c | 8 ++--
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
> index f111076..d4d12a9 100644
> --- a/kernel/sched/cputime.c
> +++ b/kernel/sched/cputime.c
> @@ -78,30 +78,26 @@ EXPORT_SYMBOL_GPL(irqtime_account_irq);
>  static cputime_t irqtime_account_hi_update(cputime_t maxtime)
>  {
>   u64 *cpustat = kcpustat_this_cpu->cpustat;
> - unsigned long flags;
>   cputime_t irq_cputime;
>  
> - local_irq_save(flags);
>   irq_cputime = nsecs_to_cputime64(__this_cpu_read(cpu_hardirq_time)) -
> cpustat[CPUTIME_IRQ];
>   irq_cputime = min(irq_cputime, maxtime);
>   cpustat[CPUTIME_IRQ] += irq_cputime;
> - local_irq_restore(flags);
> +
>   return irq_cputime;
>  }
>  
>  static cputime_t irqtime_account_si_update(cputime_t maxtime)
>  {
>   u64 *cpustat = kcpustat_this_cpu->cpustat;
> - unsigned long flags;
>   cputime_t softirq_cputime;
>  
> - local_irq_save(flags);
>   softirq_cputime = nsecs_to_cputime64(__this_cpu_read(cpu_softirq_time)) 
> -
> cpustat[CPUTIME_SOFTIRQ];
>   softirq_cputime = min(softirq_cputime, maxtime);
>   cpustat[CPUTIME_SOFTIRQ] += softirq_cputime;
> - local_irq_restore(flags);
> +
>   return softirq_cputime;
>  }
>  
> 

Re: [PATCH 1/2] ARM: dts: am57xx-beagle-x15: Remove pinmux configurations

[Nishanth Menon]

On 09/02/2016 05:18 AM, Sekhar Nori wrote:

On Friday 02 September 2016 02:35 PM, Nishanth Menon wrote:

pinmuxing for DRA7x/AM57x family of processors need to be done in IO
isolation as part of initial bootloader executed from SRAM. This is
done as part of iodelay configuration sequence and is required due to
the limitations introduced by erratum ID: i869[1] and elaborated in
the Technical Reference Manual[2] 18.4.6.1.7 Isolation Requirements.

Only peripheral that is permitted for dynamic pin mux configuration is
MMC. This is to accommodate the requirements for varied speeds (which


Actually its MMC and DCAN. DCAN because of i893 ("DCAN Initialization
Sequence"). But I can see you may have omitted reference to it since
DCAN is not really used on the x15.


True, I should add that for completeness. will do. thanks for catching 
the same.



--
Regards,
Nishanth Menon

Re: [PATCH 1/2] ARM: dts: am57xx-beagle-x15: Remove pinmux configurations

[Nishanth Menon]

On 09/02/2016 05:18 AM, Sekhar Nori wrote:

On Friday 02 September 2016 02:35 PM, Nishanth Menon wrote:

pinmuxing for DRA7x/AM57x family of processors need to be done in IO
isolation as part of initial bootloader executed from SRAM. This is
done as part of iodelay configuration sequence and is required due to
the limitations introduced by erratum ID: i869[1] and elaborated in
the Technical Reference Manual[2] 18.4.6.1.7 Isolation Requirements.

Only peripheral that is permitted for dynamic pin mux configuration is
MMC. This is to accommodate the requirements for varied speeds (which


Actually its MMC and DCAN. DCAN because of i893 ("DCAN Initialization
Sequence"). But I can see you may have omitted reference to it since
DCAN is not really used on the x15.


True, I should add that for completeness. will do. thanks for catching 
the same.



--
Regards,
Nishanth Menon

Re: [PATCH v3 2/4] dt-bindings: arm: Add Sierra Wireless modules bindings

[Rob Herring]

On Tue, Aug 23, 2016 at 01:39:04PM +0200, Neil Armstrong wrote:
> Signed-off-by: Neil Armstrong 
> ---
>  Documentation/devicetree/bindings/arm/swir.txt | 12 
>  1 file changed, 12 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/swir.txt

Acked-by: Rob Herring 

Re: [PATCH v3 2/4] dt-bindings: arm: Add Sierra Wireless modules bindings

[Rob Herring]

On Tue, Aug 23, 2016 at 01:39:04PM +0200, Neil Armstrong wrote:
> Signed-off-by: Neil Armstrong 
> ---
>  Documentation/devicetree/bindings/arm/swir.txt | 12 
>  1 file changed, 12 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/swir.txt

Acked-by: Rob Herring 

[PATCH v2 2/2] crypto: qat - fix resource release omissions

[Quentin Lambert]

In certain cases qat_uclo_parse_uof_obj used to return with an error code
before releasing all resources. This patch add a jump to the appropriate label
ensuring that the resources are properly released before returning.

This issue was found with Hector.

Signed-off-by: Quentin Lambert 
---
 drivers/crypto/qat/qat_common/qat_uclo.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -981,7 +981,8 @@ static int qat_uclo_parse_uof_obj(struct
(PID_MINOR_REV & handle->hal_handle->revision_id);
if (qat_uclo_check_uof_compat(obj_handle)) {
pr_err("QAT: UOF incompatible\n");
-   return -EINVAL;
+   ret = -EINVAL;
+   goto out_err;
}
obj_handle->ustore_phy_size = ICP_QAT_UCLO_MAX_USTORE;
if (!obj_handle->obj_hdr->file_buff ||

[PATCH v2 2/2] crypto: qat - fix resource release omissions

[Quentin Lambert]

In certain cases qat_uclo_parse_uof_obj used to return with an error code
before releasing all resources. This patch add a jump to the appropriate label
ensuring that the resources are properly released before returning.

This issue was found with Hector.

Signed-off-by: Quentin Lambert 
---
 drivers/crypto/qat/qat_common/qat_uclo.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -981,7 +981,8 @@ static int qat_uclo_parse_uof_obj(struct
(PID_MINOR_REV & handle->hal_handle->revision_id);
if (qat_uclo_check_uof_compat(obj_handle)) {
pr_err("QAT: UOF incompatible\n");
-   return -EINVAL;
+   ret = -EINVAL;
+   goto out_err;
}
obj_handle->ustore_phy_size = ICP_QAT_UCLO_MAX_USTORE;
if (!obj_handle->obj_hdr->file_buff ||

[PATCH v2 1/2] crypto: qat - introduces a variable to handle error codes

[Quentin Lambert]

Most error code used to jump to a label that lead to a "return -EFAULT"
statement. This patch introduces a variable that stores the error code
so that other error branches can use the same label to exit.

Signed-off-by: Quentin Lambert 

---
 drivers/crypto/qat/qat_common/qat_uclo.c |   13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -966,6 +966,7 @@ static int qat_uclo_parse_uof_obj(struct
 {
struct icp_qat_uclo_objhandle *obj_handle = handle->obj_handle;
unsigned int ae;
+   int ret;
 
obj_handle->uword_buf = kcalloc(UWORD_CPYBUF_SIZE, sizeof(uint64_t),
GFP_KERNEL);
@@ -987,29 +988,35 @@ static int qat_uclo_parse_uof_obj(struct
!qat_uclo_map_str_table(obj_handle->obj_hdr, ICP_QAT_UOF_STRT,
_handle->str_table)) {
pr_err("QAT: UOF doesn't have effective images\n");
+   ret = -EFAULT;
goto out_err;
}
obj_handle->uimage_num =
qat_uclo_map_uimage(obj_handle, obj_handle->ae_uimage,
ICP_QAT_UCLO_MAX_AE * ICP_QAT_UCLO_MAX_CTX);
-   if (!obj_handle->uimage_num)
+   if (!obj_handle->uimage_num) {
+   ret = -EFAULT;
goto out_err;
+   }
if (qat_uclo_map_ae(handle, handle->hal_handle->ae_max_num)) {
pr_err("QAT: Bad object\n");
+   ret = -EFAULT;
goto out_check_uof_aemask_err;
}
qat_uclo_init_uword_num(handle);
qat_uclo_map_initmem_table(_handle->encap_uof_obj,
   _handle->init_mem_tab);
-   if (qat_uclo_set_ae_mode(handle))
+   if (qat_uclo_set_ae_mode(handle)) {
+   ret = -EFAULT;
goto out_check_uof_aemask_err;
+   }
return 0;
 out_check_uof_aemask_err:
for (ae = 0; ae < obj_handle->uimage_num; ae++)
kfree(obj_handle->ae_uimage[ae].page);
 out_err:
kfree(obj_handle->uword_buf);
-   return -EFAULT;
+   return ret;
 }
 
 static int qat_uclo_map_suof_file_hdr(struct icp_qat_fw_loader_handle *handle,

[PATCH v2 1/2] crypto: qat - introduces a variable to handle error codes

[Quentin Lambert]

Most error code used to jump to a label that lead to a "return -EFAULT"
statement. This patch introduces a variable that stores the error code
so that other error branches can use the same label to exit.

Signed-off-by: Quentin Lambert 

---
 drivers/crypto/qat/qat_common/qat_uclo.c |   13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -966,6 +966,7 @@ static int qat_uclo_parse_uof_obj(struct
 {
struct icp_qat_uclo_objhandle *obj_handle = handle->obj_handle;
unsigned int ae;
+   int ret;
 
obj_handle->uword_buf = kcalloc(UWORD_CPYBUF_SIZE, sizeof(uint64_t),
GFP_KERNEL);
@@ -987,29 +988,35 @@ static int qat_uclo_parse_uof_obj(struct
!qat_uclo_map_str_table(obj_handle->obj_hdr, ICP_QAT_UOF_STRT,
_handle->str_table)) {
pr_err("QAT: UOF doesn't have effective images\n");
+   ret = -EFAULT;
goto out_err;
}
obj_handle->uimage_num =
qat_uclo_map_uimage(obj_handle, obj_handle->ae_uimage,
ICP_QAT_UCLO_MAX_AE * ICP_QAT_UCLO_MAX_CTX);
-   if (!obj_handle->uimage_num)
+   if (!obj_handle->uimage_num) {
+   ret = -EFAULT;
goto out_err;
+   }
if (qat_uclo_map_ae(handle, handle->hal_handle->ae_max_num)) {
pr_err("QAT: Bad object\n");
+   ret = -EFAULT;
goto out_check_uof_aemask_err;
}
qat_uclo_init_uword_num(handle);
qat_uclo_map_initmem_table(_handle->encap_uof_obj,
   _handle->init_mem_tab);
-   if (qat_uclo_set_ae_mode(handle))
+   if (qat_uclo_set_ae_mode(handle)) {
+   ret = -EFAULT;
goto out_check_uof_aemask_err;
+   }
return 0;
 out_check_uof_aemask_err:
for (ae = 0; ae < obj_handle->uimage_num; ae++)
kfree(obj_handle->ae_uimage[ae].page);
 out_err:
kfree(obj_handle->uword_buf);
-   return -EFAULT;
+   return ret;
 }
 
 static int qat_uclo_map_suof_file_hdr(struct icp_qat_fw_loader_handle *handle,

[PATCH v2 0/2] add omitted release in qat_common

[Quentin Lambert]

The first patch introduces a variable to handle different error codes and be
able to reuse the same clean up code. The second add an omitted release by
jumping to the clean code having set the returned value to the proper error
code.

-changes since v1
I failed to send the first version properly and it was missing a proper commit
message, just ignore it.

---
 drivers/crypto/qat/qat_common/qat_uclo.c |   16 
 1 file changed, 12 insertions(+), 4 deletions(-)
---

[PATCH v2 0/2] add omitted release in qat_common

[Quentin Lambert]

The first patch introduces a variable to handle different error codes and be
able to reuse the same clean up code. The second add an omitted release by
jumping to the clean code having set the returned value to the proper error
code.

-changes since v1
I failed to send the first version properly and it was missing a proper commit
message, just ignore it.

---
 drivers/crypto/qat/qat_common/qat_uclo.c |   16 
 1 file changed, 12 insertions(+), 4 deletions(-)
---

Re: [PATCH] x86/paravirt: Do not trace _paravirt_ident_*() functions

[Steven Rostedt]

I just spent half a day bisecting function tracing because I tripped
over this again. I thought this was merged, but I guess it was missed
again.

Can someone please pull this in. And mark it for stable, it goes
probably as far back as 2.6.32.

Thanks!

-- Steve

On Wed, 25 May 2016 13:47:26 -0400
Steven Rostedt  wrote:

> Łukasz Daniluk reported that on a RHEL kernel that his machine would lock up
> after enabling function tracer. I asked him to bisect the functions within
> available_filter_functions, which he did and it came down to three:
> 
>  _paravirt_nop(), _paravirt_ident_32() and _paravirt_ident_64()
> 
> It was found that this is only an issue when noreplace-paravirt is added to
> the kernel command line.
> 
> This means that those functions are most likely called within critical
> sections of the funtion tracer, and must not be traced.
> 
> In newer kenels _paravirt_nop() is defined within gcc asm(), and is no
> longer an issue. But both _paravirt_ident_{32,64}() causes the following
> splat when they are traced:
> 
>  mm/pgtable-generic.c:33: bad pmd 8800d2435150(01d00054)
>  mm/pgtable-generic.c:33: bad pmd 8800d3624190(01d00070)
>  mm/pgtable-generic.c:33: bad pmd 8800d36a5110(01d00054)
>  mm/pgtable-generic.c:33: bad pmd 880118eb1450(01d00054)
>  NMI watchdog: BUG: soft lockup - CPU#2 stuck for 22s! [systemd-journal:469]
>  Modules linked in: e1000e
>  CPU: 2 PID: 469 Comm: systemd-journal Not tainted 4.6.0-rc4-test+ #513
>  Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01 v02.05 
> 05/07/2012
>  task: 880118f740c0 ti: 8800d4aec000 task.ti: 8800d4aec000
>  RIP: 0010:[]  [] 
> queued_spin_lock_slowpath+0x118/0x1a0
>  RSP: 0018:8800d4aefb90  EFLAGS: 0246
>  RAX:  RBX:  RCX: 88011eb16d40
>  RDX: 82485760 RSI: 1f288820 RDI: ea008030
>  RBP: 8800d4aefb90 R08: 000c R09: 
>  R10: 821c8e0e R11:  R12: 88200fb8
>  R13: 7f7a4e3f7000 R14: ea000303f600 R15: 8800d4b562e0
>  FS:  7f7a4e3d7840() GS:88011eb0() knlGS:
>  CS:  0010 DS:  ES:  CR0: 80050033
>  CR2: 7f7a4e3f7000 CR3: d3e71000 CR4: 001406e0
>  Stack:
>   8800d4aefba0 81cc5f47 8800d4aefc60 8122c15b
>   8800d4aefcb0 8800d4aefbd0 811bf4cb 0002
>   0015 8800d2276050 8000c0fd8867 ea008030
>  Call Trace:
>   [] _raw_spin_lock+0x27/0x30
>   [] handle_pte_fault+0x13db/0x16b0
>   [] ? function_trace_call+0x15b/0x180
>   [] ? handle_pte_fault+0x5/0x16b0
>   [] handle_mm_fault+0x312/0x670
>   [] ? find_vma+0x68/0x70
>   [] __do_page_fault+0x1b1/0x4e0
>   [] do_page_fault+0x22/0x30
>   [] page_fault+0x28/0x30
>   [] ? copy_user_enhanced_fast_string+0x5/0x10
>   [] ? seq_read+0x305/0x370
>   [] __vfs_read+0x28/0xe0
>   [] ? __vfs_read+0x5/0xe0
>   [] ? __vfs_read+0x5/0xe0
>   [] vfs_read+0x86/0x130
>   [] SyS_read+0x46/0xa0
>   [] entry_SYSCALL_64_fastpath+0x1e/0xa8
>  Code: 12 48 c1 ea 0c 83 e8 01 83 e2 30 48 98 48 81 c2 40 6d 01 00 48 03 14
>  c5 80 6a 5d 82 48 89 0a 8b 41 08 85 c0 75 09 f3 90 8b 41 08 <85> c0 74 f7
>  4c 8b 09 4d 85 c9 74 08 41 0f 18 09 eb 02 f3 90 8b
> 
> Reported-by: Łukasz Daniluk 
> Signed-off-by: Steven Rostedt 
> 
> diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
> index f08ac28b8136..f975d226be6e 100644
> --- a/arch/x86/kernel/paravirt.c
> +++ b/arch/x86/kernel/paravirt.c
> @@ -55,12 +55,12 @@ asm (".pushsection .entry.text, \"ax\"\n"
>   ".popsection");
>  
>  /* identity function, which can be inlined */
> -u32 _paravirt_ident_32(u32 x)
> +u32 notrace _paravirt_ident_32(u32 x)
>  {
>   return x;
>  }
>  
> -u64 _paravirt_ident_64(u64 x)
> +u64 notrace _paravirt_ident_64(u64 x)
>  {
>   return x;
>  }

Re: [PATCH] x86/paravirt: Do not trace _paravirt_ident_*() functions

[Steven Rostedt]

I just spent half a day bisecting function tracing because I tripped
over this again. I thought this was merged, but I guess it was missed
again.

Can someone please pull this in. And mark it for stable, it goes
probably as far back as 2.6.32.

Thanks!

-- Steve

On Wed, 25 May 2016 13:47:26 -0400
Steven Rostedt  wrote:

> Łukasz Daniluk reported that on a RHEL kernel that his machine would lock up
> after enabling function tracer. I asked him to bisect the functions within
> available_filter_functions, which he did and it came down to three:
> 
>  _paravirt_nop(), _paravirt_ident_32() and _paravirt_ident_64()
> 
> It was found that this is only an issue when noreplace-paravirt is added to
> the kernel command line.
> 
> This means that those functions are most likely called within critical
> sections of the funtion tracer, and must not be traced.
> 
> In newer kenels _paravirt_nop() is defined within gcc asm(), and is no
> longer an issue. But both _paravirt_ident_{32,64}() causes the following
> splat when they are traced:
> 
>  mm/pgtable-generic.c:33: bad pmd 8800d2435150(01d00054)
>  mm/pgtable-generic.c:33: bad pmd 8800d3624190(01d00070)
>  mm/pgtable-generic.c:33: bad pmd 8800d36a5110(01d00054)
>  mm/pgtable-generic.c:33: bad pmd 880118eb1450(01d00054)
>  NMI watchdog: BUG: soft lockup - CPU#2 stuck for 22s! [systemd-journal:469]
>  Modules linked in: e1000e
>  CPU: 2 PID: 469 Comm: systemd-journal Not tainted 4.6.0-rc4-test+ #513
>  Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01 v02.05 
> 05/07/2012
>  task: 880118f740c0 ti: 8800d4aec000 task.ti: 8800d4aec000
>  RIP: 0010:[]  [] 
> queued_spin_lock_slowpath+0x118/0x1a0
>  RSP: 0018:8800d4aefb90  EFLAGS: 0246
>  RAX:  RBX:  RCX: 88011eb16d40
>  RDX: 82485760 RSI: 1f288820 RDI: ea008030
>  RBP: 8800d4aefb90 R08: 000c R09: 
>  R10: 821c8e0e R11:  R12: 88200fb8
>  R13: 7f7a4e3f7000 R14: ea000303f600 R15: 8800d4b562e0
>  FS:  7f7a4e3d7840() GS:88011eb0() knlGS:
>  CS:  0010 DS:  ES:  CR0: 80050033
>  CR2: 7f7a4e3f7000 CR3: d3e71000 CR4: 001406e0
>  Stack:
>   8800d4aefba0 81cc5f47 8800d4aefc60 8122c15b
>   8800d4aefcb0 8800d4aefbd0 811bf4cb 0002
>   0015 8800d2276050 8000c0fd8867 ea008030
>  Call Trace:
>   [] _raw_spin_lock+0x27/0x30
>   [] handle_pte_fault+0x13db/0x16b0
>   [] ? function_trace_call+0x15b/0x180
>   [] ? handle_pte_fault+0x5/0x16b0
>   [] handle_mm_fault+0x312/0x670
>   [] ? find_vma+0x68/0x70
>   [] __do_page_fault+0x1b1/0x4e0
>   [] do_page_fault+0x22/0x30
>   [] page_fault+0x28/0x30
>   [] ? copy_user_enhanced_fast_string+0x5/0x10
>   [] ? seq_read+0x305/0x370
>   [] __vfs_read+0x28/0xe0
>   [] ? __vfs_read+0x5/0xe0
>   [] ? __vfs_read+0x5/0xe0
>   [] vfs_read+0x86/0x130
>   [] SyS_read+0x46/0xa0
>   [] entry_SYSCALL_64_fastpath+0x1e/0xa8
>  Code: 12 48 c1 ea 0c 83 e8 01 83 e2 30 48 98 48 81 c2 40 6d 01 00 48 03 14
>  c5 80 6a 5d 82 48 89 0a 8b 41 08 85 c0 75 09 f3 90 8b 41 08 <85> c0 74 f7
>  4c 8b 09 4d 85 c9 74 08 41 0f 18 09 eb 02 f3 90 8b
> 
> Reported-by: Łukasz Daniluk 
> Signed-off-by: Steven Rostedt 
> 
> diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
> index f08ac28b8136..f975d226be6e 100644
> --- a/arch/x86/kernel/paravirt.c
> +++ b/arch/x86/kernel/paravirt.c
> @@ -55,12 +55,12 @@ asm (".pushsection .entry.text, \"ax\"\n"
>   ".popsection");
>  
>  /* identity function, which can be inlined */
> -u32 _paravirt_ident_32(u32 x)
> +u32 notrace _paravirt_ident_32(u32 x)
>  {
>   return x;
>  }
>  
> -u64 _paravirt_ident_64(u64 x)
> +u64 notrace _paravirt_ident_64(u64 x)
>  {
>   return x;
>  }

Re: [PATCH 1/2] sdhci-of-arasan: Add device tree parameter fails-without-test-cd bit

[Rob Herring]

On Mon, Aug 29, 2016 at 06:20:56PM -0500, Zach Brown wrote:
> The sdhci controller on xilinx zynq devices will not function unless
> the CD bit is provided. http://www.xilinx.com/support/answers/61064.html
> In cases where it is impossible to provide the CD bit in hardware,
> setting the controller to test mode and then setting inserted to true
> will get the controller to function without the CD bit.
> 
> The device property "fails-without-test-cd" will let the arasan driver know
> the controller does not have the CD line wired and that the controller
> does not function without it.
> 
> Signed-off-by: Zach Brown 
> ---
> v2:
>  * improved commit messages
>  * removed fake-cd device property
>  * removed fake-cd quirk
>  * use broken-cd device property
>  * documented new usage of broken-cd
> v3:
>  * removed new usage of broken-cd
>  * created fails-without-test-cd device property
>  * created arasan controller specific quirk
> 
> 
>  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 3404afa..ec0dc82 100644
> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> @@ -21,6 +21,10 @@ Required Properties:
>- interrupts: Interrupt specifier
>- interrupt-parent: Phandle for the interrupt controller that services
> interrupts for this device.
> +Optional Properties:
> +- fails-without-test-cd: when present, the controller doesn't work when the 
> CD
> +  line is not connected properly, and the line is not connected properly. 
> Test
> +  mode can be used to force the controller to function.

Prefix with either arasan or xlnx. Seems more likely Xilinx specific to 
me.

Rob

Re: [PATCH 1/2] sdhci-of-arasan: Add device tree parameter fails-without-test-cd bit

[Rob Herring]

On Mon, Aug 29, 2016 at 06:20:56PM -0500, Zach Brown wrote:
> The sdhci controller on xilinx zynq devices will not function unless
> the CD bit is provided. http://www.xilinx.com/support/answers/61064.html
> In cases where it is impossible to provide the CD bit in hardware,
> setting the controller to test mode and then setting inserted to true
> will get the controller to function without the CD bit.
> 
> The device property "fails-without-test-cd" will let the arasan driver know
> the controller does not have the CD line wired and that the controller
> does not function without it.
> 
> Signed-off-by: Zach Brown 
> ---
> v2:
>  * improved commit messages
>  * removed fake-cd device property
>  * removed fake-cd quirk
>  * use broken-cd device property
>  * documented new usage of broken-cd
> v3:
>  * removed new usage of broken-cd
>  * created fails-without-test-cd device property
>  * created arasan controller specific quirk
> 
> 
>  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 3404afa..ec0dc82 100644
> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> @@ -21,6 +21,10 @@ Required Properties:
>- interrupts: Interrupt specifier
>- interrupt-parent: Phandle for the interrupt controller that services
> interrupts for this device.
> +Optional Properties:
> +- fails-without-test-cd: when present, the controller doesn't work when the 
> CD
> +  line is not connected properly, and the line is not connected properly. 
> Test
> +  mode can be used to force the controller to function.

Prefix with either arasan or xlnx. Seems more likely Xilinx specific to 
me.

Rob

[PATCH 1/2] crypto: qat - introduces a variable to handle error codes

[Quentin Lambert]

Most error code used to jump to a label that lead to a "return -EFAULT"
statement. This patch introduces a variable that stores the error code
so that other error branches can use the same label to exit.

Signed-off-by: Quentin Lambert 

---
 drivers/crypto/qat/qat_common/qat_uclo.c |   13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -966,6 +966,7 @@ static int qat_uclo_parse_uof_obj(struct
 {
struct icp_qat_uclo_objhandle *obj_handle = handle->obj_handle;
unsigned int ae;
+   int ret;
 
obj_handle->uword_buf = kcalloc(UWORD_CPYBUF_SIZE, sizeof(uint64_t),
GFP_KERNEL);
@@ -987,29 +988,35 @@ static int qat_uclo_parse_uof_obj(struct
!qat_uclo_map_str_table(obj_handle->obj_hdr, ICP_QAT_UOF_STRT,
_handle->str_table)) {
pr_err("QAT: UOF doesn't have effective images\n");
+   ret = -EFAULT;
goto out_err;
}
obj_handle->uimage_num =
qat_uclo_map_uimage(obj_handle, obj_handle->ae_uimage,
ICP_QAT_UCLO_MAX_AE * ICP_QAT_UCLO_MAX_CTX);
-   if (!obj_handle->uimage_num)
+   if (!obj_handle->uimage_num) {
+   ret = -EFAULT;
goto out_err;
+   }
if (qat_uclo_map_ae(handle, handle->hal_handle->ae_max_num)) {
pr_err("QAT: Bad object\n");
+   ret = -EFAULT;
goto out_check_uof_aemask_err;
}
qat_uclo_init_uword_num(handle);
qat_uclo_map_initmem_table(_handle->encap_uof_obj,
   _handle->init_mem_tab);
-   if (qat_uclo_set_ae_mode(handle))
+   if (qat_uclo_set_ae_mode(handle)) {
+   ret = -EFAULT;
goto out_check_uof_aemask_err;
+   }
return 0;
 out_check_uof_aemask_err:
for (ae = 0; ae < obj_handle->uimage_num; ae++)
kfree(obj_handle->ae_uimage[ae].page);
 out_err:
kfree(obj_handle->uword_buf);
-   return -EFAULT;
+   return ret;
 }
 
 static int qat_uclo_map_suof_file_hdr(struct icp_qat_fw_loader_handle *handle,

[PATCH 1/2] crypto: qat - introduces a variable to handle error codes

[Quentin Lambert]

Most error code used to jump to a label that lead to a "return -EFAULT"
statement. This patch introduces a variable that stores the error code
so that other error branches can use the same label to exit.

Signed-off-by: Quentin Lambert 

---
 drivers/crypto/qat/qat_common/qat_uclo.c |   13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -966,6 +966,7 @@ static int qat_uclo_parse_uof_obj(struct
 {
struct icp_qat_uclo_objhandle *obj_handle = handle->obj_handle;
unsigned int ae;
+   int ret;
 
obj_handle->uword_buf = kcalloc(UWORD_CPYBUF_SIZE, sizeof(uint64_t),
GFP_KERNEL);
@@ -987,29 +988,35 @@ static int qat_uclo_parse_uof_obj(struct
!qat_uclo_map_str_table(obj_handle->obj_hdr, ICP_QAT_UOF_STRT,
_handle->str_table)) {
pr_err("QAT: UOF doesn't have effective images\n");
+   ret = -EFAULT;
goto out_err;
}
obj_handle->uimage_num =
qat_uclo_map_uimage(obj_handle, obj_handle->ae_uimage,
ICP_QAT_UCLO_MAX_AE * ICP_QAT_UCLO_MAX_CTX);
-   if (!obj_handle->uimage_num)
+   if (!obj_handle->uimage_num) {
+   ret = -EFAULT;
goto out_err;
+   }
if (qat_uclo_map_ae(handle, handle->hal_handle->ae_max_num)) {
pr_err("QAT: Bad object\n");
+   ret = -EFAULT;
goto out_check_uof_aemask_err;
}
qat_uclo_init_uword_num(handle);
qat_uclo_map_initmem_table(_handle->encap_uof_obj,
   _handle->init_mem_tab);
-   if (qat_uclo_set_ae_mode(handle))
+   if (qat_uclo_set_ae_mode(handle)) {
+   ret = -EFAULT;
goto out_check_uof_aemask_err;
+   }
return 0;
 out_check_uof_aemask_err:
for (ae = 0; ae < obj_handle->uimage_num; ae++)
kfree(obj_handle->ae_uimage[ae].page);
 out_err:
kfree(obj_handle->uword_buf);
-   return -EFAULT;
+   return ret;
 }
 
 static int qat_uclo_map_suof_file_hdr(struct icp_qat_fw_loader_handle *handle,

[PATCH 2/2] crypto: qat - fix resource release omissions

[Quentin Lambert]

This issue was found with Hector.

Signed-off-by: Quentin Lambert 
---
 drivers/crypto/qat/qat_common/qat_uclo.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -981,7 +981,8 @@ static int qat_uclo_parse_uof_obj(struct
(PID_MINOR_REV & handle->hal_handle->revision_id);
if (qat_uclo_check_uof_compat(obj_handle)) {
pr_err("QAT: UOF incompatible\n");
-   return -EINVAL;
+   ret = -EINVAL;
+   goto out_err;
}
obj_handle->ustore_phy_size = ICP_QAT_UCLO_MAX_USTORE;
if (!obj_handle->obj_hdr->file_buff ||

[PATCH 2/2] crypto: qat - fix resource release omissions

[Quentin Lambert]

This issue was found with Hector.

Signed-off-by: Quentin Lambert 
---
 drivers/crypto/qat/qat_common/qat_uclo.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -981,7 +981,8 @@ static int qat_uclo_parse_uof_obj(struct
(PID_MINOR_REV & handle->hal_handle->revision_id);
if (qat_uclo_check_uof_compat(obj_handle)) {
pr_err("QAT: UOF incompatible\n");
-   return -EINVAL;
+   ret = -EINVAL;
+   goto out_err;
}
obj_handle->ustore_phy_size = ICP_QAT_UCLO_MAX_USTORE;
if (!obj_handle->obj_hdr->file_buff ||

[PATCH 1/2][RESEND] crypto: qat - introduces a variable to handle error codes

[Quentin Lambert]

Most error code used to jump to a label that lead to a "return -EFAULT"
statement. This patch introduces a variable that stores the error code
so that other error branches can use the same label to exit.

Signed-off-by: Quentin Lambert 

---
 drivers/crypto/qat/qat_common/qat_uclo.c |   13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -966,6 +966,7 @@ static int qat_uclo_parse_uof_obj(struct
 {
struct icp_qat_uclo_objhandle *obj_handle = handle->obj_handle;
unsigned int ae;
+   int ret;
 
obj_handle->uword_buf = kcalloc(UWORD_CPYBUF_SIZE, sizeof(uint64_t),
GFP_KERNEL);
@@ -987,29 +988,35 @@ static int qat_uclo_parse_uof_obj(struct
!qat_uclo_map_str_table(obj_handle->obj_hdr, ICP_QAT_UOF_STRT,
_handle->str_table)) {
pr_err("QAT: UOF doesn't have effective images\n");
+   ret = -EFAULT;
goto out_err;
}
obj_handle->uimage_num =
qat_uclo_map_uimage(obj_handle, obj_handle->ae_uimage,
ICP_QAT_UCLO_MAX_AE * ICP_QAT_UCLO_MAX_CTX);
-   if (!obj_handle->uimage_num)
+   if (!obj_handle->uimage_num) {
+   ret = -EFAULT;
goto out_err;
+   }
if (qat_uclo_map_ae(handle, handle->hal_handle->ae_max_num)) {
pr_err("QAT: Bad object\n");
+   ret = -EFAULT;
goto out_check_uof_aemask_err;
}
qat_uclo_init_uword_num(handle);
qat_uclo_map_initmem_table(_handle->encap_uof_obj,
   _handle->init_mem_tab);
-   if (qat_uclo_set_ae_mode(handle))
+   if (qat_uclo_set_ae_mode(handle)) {
+   ret = -EFAULT;
goto out_check_uof_aemask_err;
+   }
return 0;
 out_check_uof_aemask_err:
for (ae = 0; ae < obj_handle->uimage_num; ae++)
kfree(obj_handle->ae_uimage[ae].page);
 out_err:
kfree(obj_handle->uword_buf);
-   return -EFAULT;
+   return ret;
 }
 
 static int qat_uclo_map_suof_file_hdr(struct icp_qat_fw_loader_handle *handle,

[PATCH 0/2][RESEND] add omitted release in qat_common

[Quentin Lambert]

The first patch introduces a variable to handle different error codes and be
able to reuse the same clean up code. The second add an omitted release by
jumping to the clean code having set the returned value to the proper error
code.

---
 drivers/crypto/qat/qat_common/qat_uclo.c |   16 
 1 file changed, 12 insertions(+), 4 deletions(-)
---

[PATCH 2/2] crypto: qat - fix resource release omissions

[Quentin Lambert]

In certain cases qat_uclo_parse_uof_obj used to return with an error code
before releasing all resources. This patch add a jump to the appropriate label
ensuring that the resources are properly released before returning.

This issue was found with Hector.

Signed-off-by: Quentin Lambert 
---
 drivers/crypto/qat/qat_common/qat_uclo.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -981,7 +981,8 @@ static int qat_uclo_parse_uof_obj(struct
(PID_MINOR_REV & handle->hal_handle->revision_id);
if (qat_uclo_check_uof_compat(obj_handle)) {
pr_err("QAT: UOF incompatible\n");
-   return -EINVAL;
+   ret = -EINVAL;
+   goto out_err;
}
obj_handle->ustore_phy_size = ICP_QAT_UCLO_MAX_USTORE;
if (!obj_handle->obj_hdr->file_buff ||

[PATCH 1/2][RESEND] crypto: qat - introduces a variable to handle error codes

[Quentin Lambert]

Most error code used to jump to a label that lead to a "return -EFAULT"
statement. This patch introduces a variable that stores the error code
so that other error branches can use the same label to exit.

Signed-off-by: Quentin Lambert 

---
 drivers/crypto/qat/qat_common/qat_uclo.c |   13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -966,6 +966,7 @@ static int qat_uclo_parse_uof_obj(struct
 {
struct icp_qat_uclo_objhandle *obj_handle = handle->obj_handle;
unsigned int ae;
+   int ret;
 
obj_handle->uword_buf = kcalloc(UWORD_CPYBUF_SIZE, sizeof(uint64_t),
GFP_KERNEL);
@@ -987,29 +988,35 @@ static int qat_uclo_parse_uof_obj(struct
!qat_uclo_map_str_table(obj_handle->obj_hdr, ICP_QAT_UOF_STRT,
_handle->str_table)) {
pr_err("QAT: UOF doesn't have effective images\n");
+   ret = -EFAULT;
goto out_err;
}
obj_handle->uimage_num =
qat_uclo_map_uimage(obj_handle, obj_handle->ae_uimage,
ICP_QAT_UCLO_MAX_AE * ICP_QAT_UCLO_MAX_CTX);
-   if (!obj_handle->uimage_num)
+   if (!obj_handle->uimage_num) {
+   ret = -EFAULT;
goto out_err;
+   }
if (qat_uclo_map_ae(handle, handle->hal_handle->ae_max_num)) {
pr_err("QAT: Bad object\n");
+   ret = -EFAULT;
goto out_check_uof_aemask_err;
}
qat_uclo_init_uword_num(handle);
qat_uclo_map_initmem_table(_handle->encap_uof_obj,
   _handle->init_mem_tab);
-   if (qat_uclo_set_ae_mode(handle))
+   if (qat_uclo_set_ae_mode(handle)) {
+   ret = -EFAULT;
goto out_check_uof_aemask_err;
+   }
return 0;
 out_check_uof_aemask_err:
for (ae = 0; ae < obj_handle->uimage_num; ae++)
kfree(obj_handle->ae_uimage[ae].page);
 out_err:
kfree(obj_handle->uword_buf);
-   return -EFAULT;
+   return ret;
 }
 
 static int qat_uclo_map_suof_file_hdr(struct icp_qat_fw_loader_handle *handle,

[PATCH 0/2][RESEND] add omitted release in qat_common

[Quentin Lambert]

The first patch introduces a variable to handle different error codes and be
able to reuse the same clean up code. The second add an omitted release by
jumping to the clean code having set the returned value to the proper error
code.

---
 drivers/crypto/qat/qat_common/qat_uclo.c |   16 
 1 file changed, 12 insertions(+), 4 deletions(-)
---

[PATCH 2/2] crypto: qat - fix resource release omissions

[Quentin Lambert]

In certain cases qat_uclo_parse_uof_obj used to return with an error code
before releasing all resources. This patch add a jump to the appropriate label
ensuring that the resources are properly released before returning.

This issue was found with Hector.

Signed-off-by: Quentin Lambert 
---
 drivers/crypto/qat/qat_common/qat_uclo.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -981,7 +981,8 @@ static int qat_uclo_parse_uof_obj(struct
(PID_MINOR_REV & handle->hal_handle->revision_id);
if (qat_uclo_check_uof_compat(obj_handle)) {
pr_err("QAT: UOF incompatible\n");
-   return -EINVAL;
+   ret = -EINVAL;
+   goto out_err;
}
obj_handle->ustore_phy_size = ICP_QAT_UCLO_MAX_USTORE;
if (!obj_handle->obj_hdr->file_buff ||

Re: [PATCH 6/6] Documentation: devicetree: dwc2: Deprecate g-tx-fifo-size

[Rob Herring]

On Mon, Aug 29, 2016 at 01:39:03PM -0700, John Youn wrote:
> This property is not needed because the periodic fifos are not
> configurable. So it was incorrect to add this property in the first
> place.
> 
> Signed-off-by: John Youn 
> ---
>  Documentation/devicetree/bindings/usb/dwc2.txt | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)

Acked-by: Rob Herring 

Re: [PATCH 6/6] Documentation: devicetree: dwc2: Deprecate g-tx-fifo-size

[Rob Herring]

On Mon, Aug 29, 2016 at 01:39:03PM -0700, John Youn wrote:
> This property is not needed because the periodic fifos are not
> configurable. So it was incorrect to add this property in the first
> place.
> 
> Signed-off-by: John Youn 
> ---
>  Documentation/devicetree/bindings/usb/dwc2.txt | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)

Acked-by: Rob Herring 

Re: [PATCH V4] leds: trigger: Introduce an USB port trigger

[Alan Stern]

On Fri, 2 Sep 2016, Jacek Anaszewski wrote:

> >>> I'm pretty sure noone ever planned to have more than 1 trigger
> >>> assigned to a single LED. I just realized there will be a problem with
> >>> proposed solution: sysfs files conflict.

...

> >> Currently we support only triggers dedicated to specific type of
> >> devices. Even in case of triggers that don't expose custom sysfs
> >> attributes, registered with led_trigger_register_simple(), device
> >> drivers have to generate trigger event with dedicated function, e.g:
> >>
> >> - ledtrig-cpu: void ledtrig_cpu(enum cpu_led_event ledevt)
> >> - ledtrig-disk: void ledtrig_disk_activity(void)
> >> - ledtrig-mtd: void ledtrig_mtd_activity(void)
> >>
> >> If one wanted to have the LED notified by different type of devices,
> >> then they would have to implement a trigger that would exposed all
> >> required types of API.
> >>
> >> Unfortunately, there are many possible combinations of
> >> triggers and that doesn't sound sane to add a new one when someone
> >> will find it useful. Of course it would entail adding a call to the
> >> new trigger API in the drivers, which doesn't seem like something
> >> acceptable in the mainline.
> >
> > Maybe it would make more sense, in this case, to allow only three
> > possibilities for a USB port activity trigger.  Toggle the LED
> > whenever:
> >
> > There is activity on the specified port, or
> >
> > There is any activity on any port on the specified hub, or
> >
> > There is any USB activity on any port.
> >
> > That ought to cover most of the normal use cases, and it would be
> > simple enough to implement.
> 
> What would be the benefit of having a USB port activity trigger,
> for which we would be specifying the port to observe, but in the same
> time we would react on any activity on any port (cases 1 and 3)?

I meant these three cases to be mutually exclusive.  For a given LED,
you could have only one of those trigger types (like mentioned above,
only one trigger per LED).  For example, you might accept any one of:

echo usb1-4.2 >/sys/class/led/foo/trigger

echo hub1-4 >/sys/class/led/foo/trigger

echo usb >/sys/class/led/foo/trigger

Yes, it would be possible to have a port-specific trigger for one LED
and an overall USB activity trigger for another LED.  I don't know how
useful this would be -- you could probably imagine some unlikely
scenario.

The point is that doing things this way wouldn't require any API
violations, and it would allow users to do almost all of the things
they are likely to want.

Alan Stern

Re: [PATCH V4] leds: trigger: Introduce an USB port trigger

[Alan Stern]

On Fri, 2 Sep 2016, Jacek Anaszewski wrote:

> >>> I'm pretty sure noone ever planned to have more than 1 trigger
> >>> assigned to a single LED. I just realized there will be a problem with
> >>> proposed solution: sysfs files conflict.

...

> >> Currently we support only triggers dedicated to specific type of
> >> devices. Even in case of triggers that don't expose custom sysfs
> >> attributes, registered with led_trigger_register_simple(), device
> >> drivers have to generate trigger event with dedicated function, e.g:
> >>
> >> - ledtrig-cpu: void ledtrig_cpu(enum cpu_led_event ledevt)
> >> - ledtrig-disk: void ledtrig_disk_activity(void)
> >> - ledtrig-mtd: void ledtrig_mtd_activity(void)
> >>
> >> If one wanted to have the LED notified by different type of devices,
> >> then they would have to implement a trigger that would exposed all
> >> required types of API.
> >>
> >> Unfortunately, there are many possible combinations of
> >> triggers and that doesn't sound sane to add a new one when someone
> >> will find it useful. Of course it would entail adding a call to the
> >> new trigger API in the drivers, which doesn't seem like something
> >> acceptable in the mainline.
> >
> > Maybe it would make more sense, in this case, to allow only three
> > possibilities for a USB port activity trigger.  Toggle the LED
> > whenever:
> >
> > There is activity on the specified port, or
> >
> > There is any activity on any port on the specified hub, or
> >
> > There is any USB activity on any port.
> >
> > That ought to cover most of the normal use cases, and it would be
> > simple enough to implement.
> 
> What would be the benefit of having a USB port activity trigger,
> for which we would be specifying the port to observe, but in the same
> time we would react on any activity on any port (cases 1 and 3)?

I meant these three cases to be mutually exclusive.  For a given LED,
you could have only one of those trigger types (like mentioned above,
only one trigger per LED).  For example, you might accept any one of:

echo usb1-4.2 >/sys/class/led/foo/trigger

echo hub1-4 >/sys/class/led/foo/trigger

echo usb >/sys/class/led/foo/trigger

Yes, it would be possible to have a port-specific trigger for one LED
and an overall USB activity trigger for another LED.  I don't know how
useful this would be -- you could probably imagine some unlikely
scenario.

The point is that doing things this way wouldn't require any API
violations, and it would allow users to do almost all of the things
they are likely to want.

Alan Stern

Re: [PATCH 3/5] u64_stats: Introduce IRQs disabled helpers

[Paolo Bonzini]

On 02/09/2016 16:03, Frederic Weisbecker wrote:
>  static inline unsigned int u64_stats_fetch_begin(const struct u64_stats_sync 
> *syncp)
>  {
> -#if BITS_PER_LONG==32 && defined(CONFIG_SMP)
> - return read_seqcount_begin(>seq);
> -#else
> -#if BITS_PER_LONG==32
> +#if BITS_PER_LONG==32 && !defined(CONFIG_SMP)
>   preempt_disable();
> +#else

This should be #endif, or this side ends without a "return" statement.

> + return __u64_stats_fetch_begin(syncp);
>  #endif
> - return 0;
> +}

...

> 
>  static inline bool u64_stats_fetch_retry(const struct u64_stats_sync *syncp,
>unsigned int start)
>  {
> -#if BITS_PER_LONG==32 && defined(CONFIG_SMP)
> - return read_seqcount_retry(>seq, start);
> -#else
> -#if BITS_PER_LONG==32
> +#if BITS_PER_LONG==32 && !defined(CONFIG_SMP)
>   preempt_enable();
> -#endif
> - return false;
> +#else

Same here.

> + return __u64_stats_fetch_retry(syncp, start);
>  #endif
>  }


...

> 
> - return read_seqcount_begin(>seq);
> -#else
> -#if BITS_PER_LONG==32
> +#if BITS_PER_LONG==32 && !defined(CONFIG_SMP)
>   local_irq_disable();
> -#endif
> - return 0;
> +#else

Same here.

> + return __u64_stats_fetch_begin(syncp);
>  #endif


> 
> -#if BITS_PER_LONG==32 && defined(CONFIG_SMP)
> - return read_seqcount_retry(>seq, start);
> -#else
> -#if BITS_PER_LONG==32
> +#if BITS_PER_LONG==32 && !defined(CONFIG_SMP)
>   local_irq_enable();
> -#endif
> - return false;
> +#else

Same here.

> + return __u64_stats_fetch_retry(syncp, start);
>  #endif


Thanks,

Paolo

Re: [PATCH 3/5] u64_stats: Introduce IRQs disabled helpers

[Paolo Bonzini]

On 02/09/2016 16:03, Frederic Weisbecker wrote:
>  static inline unsigned int u64_stats_fetch_begin(const struct u64_stats_sync 
> *syncp)
>  {
> -#if BITS_PER_LONG==32 && defined(CONFIG_SMP)
> - return read_seqcount_begin(>seq);
> -#else
> -#if BITS_PER_LONG==32
> +#if BITS_PER_LONG==32 && !defined(CONFIG_SMP)
>   preempt_disable();
> +#else

This should be #endif, or this side ends without a "return" statement.

> + return __u64_stats_fetch_begin(syncp);
>  #endif
> - return 0;
> +}

...

> 
>  static inline bool u64_stats_fetch_retry(const struct u64_stats_sync *syncp,
>unsigned int start)
>  {
> -#if BITS_PER_LONG==32 && defined(CONFIG_SMP)
> - return read_seqcount_retry(>seq, start);
> -#else
> -#if BITS_PER_LONG==32
> +#if BITS_PER_LONG==32 && !defined(CONFIG_SMP)
>   preempt_enable();
> -#endif
> - return false;
> +#else

Same here.

> + return __u64_stats_fetch_retry(syncp, start);
>  #endif
>  }


...

> 
> - return read_seqcount_begin(>seq);
> -#else
> -#if BITS_PER_LONG==32
> +#if BITS_PER_LONG==32 && !defined(CONFIG_SMP)
>   local_irq_disable();
> -#endif
> - return 0;
> +#else

Same here.

> + return __u64_stats_fetch_begin(syncp);
>  #endif


> 
> -#if BITS_PER_LONG==32 && defined(CONFIG_SMP)
> - return read_seqcount_retry(>seq, start);
> -#else
> -#if BITS_PER_LONG==32
> +#if BITS_PER_LONG==32 && !defined(CONFIG_SMP)
>   local_irq_enable();
> -#endif
> - return false;
> +#else

Same here.

> + return __u64_stats_fetch_retry(syncp, start);
>  #endif


Thanks,

Paolo

Re: [PATCH 1/2] IB/hfi1: Fix a parameter of find_first_bit.

[Doug Ledford]

On 8/28/2016 2:06 AM, Leon Romanovsky wrote:
> On Fri, Aug 26, 2016 at 03:34:48PM -0400, Doug Ledford wrote:
>> On 8/26/2016 3:29 PM, Leon Romanovsky wrote:
>>> On Fri, Aug 26, 2016 at 02:01:55PM -0400, Doug Ledford wrote:
 On 8/26/2016 9:35 AM, Doug Ledford wrote:
> On 8/26/2016 12:49 AM, Christophe JAILLET wrote:
>> The 2nd parameter of 'find_first_bit' is the number of bits to search.
>> In this case, we are passing 'sizeof(unsigned long)' which is likely to
>> be 4 or 8.
>
> If the size can be 4 or 8, then using 64 universally is not correct.
> Why not use sizeof() * 8 (or << 3)?

 Better yet, why not put this patch in the kernel first:

 diff --git a/include/linux/kernel.h b/include/linux/kernel.h
 index d96a6118d26a..a8838c87668e 100644
 --- a/include/linux/kernel.h
 +++ b/include/linux/kernel.h
 @@ -52,6 +52,8 @@

  #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) +
 __must_be_array(arr))

 +#define bitsizeof(x)   (sizeof((x)) << 3)
 +
  #define u64_to_user_ptr(x) (   \
  {  \
 typecheck(u64, x);  \

 then start going around replacing all these hard coded numbers with the
 use of bitsizeof().  It can be applied not just to the find_first*bit()
 routines, but to a bunch of other routines too.  Just look at
 include/linux/bitmap.h and any that have nbits as an argument are
 candidates.
>>>
>>> There is BITS_PER_LONG define for that. There is actual use of it in mlx5 
>>> for
>>> the similar code pieces.
>>
>> BITS_PER_LONG only works if your bitfield is a single long.  It doesn't
>> work for other bitfields.  What I posted above will work for anything.
> 
> Yes, the question to ask if it is really needed.

A quick review of the usage of find_first_bit in the kernel shows that
the majority of uses that use large, complex bit arrays (things larger
than a single long, where bitsizeof(complex_object) might be helpful),
also tend to have limits to their bitmap sizes that do not directly
equal the size of the bitmap.  For example, the bitmap for an md raid
array is allocated as a chunk of memory, where the chunk of memory is
larger than the bitmap size as a general rule, so
bitsizeof(chunk_of_memory) would run off the end of the real bitmap.
Likewise for a number of other complex bitmaps (block layer tag in use
bitmap for instance).

So, is it useful?  I think so.  But it's not needed for original patch
in this thread.


-- 
Doug Ledford 
GPG Key ID: 0E572FDD



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 1/2] IB/hfi1: Fix a parameter of find_first_bit.

[Doug Ledford]

On 8/28/2016 2:06 AM, Leon Romanovsky wrote:
> On Fri, Aug 26, 2016 at 03:34:48PM -0400, Doug Ledford wrote:
>> On 8/26/2016 3:29 PM, Leon Romanovsky wrote:
>>> On Fri, Aug 26, 2016 at 02:01:55PM -0400, Doug Ledford wrote:
 On 8/26/2016 9:35 AM, Doug Ledford wrote:
> On 8/26/2016 12:49 AM, Christophe JAILLET wrote:
>> The 2nd parameter of 'find_first_bit' is the number of bits to search.
>> In this case, we are passing 'sizeof(unsigned long)' which is likely to
>> be 4 or 8.
>
> If the size can be 4 or 8, then using 64 universally is not correct.
> Why not use sizeof() * 8 (or << 3)?

 Better yet, why not put this patch in the kernel first:

 diff --git a/include/linux/kernel.h b/include/linux/kernel.h
 index d96a6118d26a..a8838c87668e 100644
 --- a/include/linux/kernel.h
 +++ b/include/linux/kernel.h
 @@ -52,6 +52,8 @@

  #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) +
 __must_be_array(arr))

 +#define bitsizeof(x)   (sizeof((x)) << 3)
 +
  #define u64_to_user_ptr(x) (   \
  {  \
 typecheck(u64, x);  \

 then start going around replacing all these hard coded numbers with the
 use of bitsizeof().  It can be applied not just to the find_first*bit()
 routines, but to a bunch of other routines too.  Just look at
 include/linux/bitmap.h and any that have nbits as an argument are
 candidates.
>>>
>>> There is BITS_PER_LONG define for that. There is actual use of it in mlx5 
>>> for
>>> the similar code pieces.
>>
>> BITS_PER_LONG only works if your bitfield is a single long.  It doesn't
>> work for other bitfields.  What I posted above will work for anything.
> 
> Yes, the question to ask if it is really needed.

A quick review of the usage of find_first_bit in the kernel shows that
the majority of uses that use large, complex bit arrays (things larger
than a single long, where bitsizeof(complex_object) might be helpful),
also tend to have limits to their bitmap sizes that do not directly
equal the size of the bitmap.  For example, the bitmap for an md raid
array is allocated as a chunk of memory, where the chunk of memory is
larger than the bitmap size as a general rule, so
bitsizeof(chunk_of_memory) would run off the end of the real bitmap.
Likewise for a number of other complex bitmaps (block layer tag in use
bitmap for instance).

So, is it useful?  I think so.  But it's not needed for original patch
in this thread.


-- 
Doug Ledford 
GPG Key ID: 0E572FDD



signature.asc
Description: OpenPGP digital signature

[PATCH 0/2] add omitted release in qat_common

[Quentin Lambert]

The first patch introduces a variable to handle different error codes and be
able to reuse the same clean up code. The second add an omitted release by
jumping to the clean code having set the returned value to the proper error
code.

---
 drivers/crypto/qat/qat_common/qat_uclo.c |   16 
 1 file changed, 12 insertions(+), 4 deletions(-)
---

[PATCH 0/2] add omitted release in qat_common

[Quentin Lambert]

The first patch introduces a variable to handle different error codes and be
able to reuse the same clean up code. The second add an omitted release by
jumping to the clean code having set the returned value to the proper error
code.

---
 drivers/crypto/qat/qat_common/qat_uclo.c |   16 
 1 file changed, 12 insertions(+), 4 deletions(-)
---

Re: [PATCH 1/3] Documentation: dt: Add TI-SCI PM Domains

[Rob Herring]

On Fri, Aug 19, 2016 at 06:56:51PM -0500, Nishanth Menon wrote:
> From: Dave Gerlach 
> 
> Add a generic power domain implementation, TI SCI PM Domains, that
> will hook into the genpd framework and allow each PD, which will be
> created one per device, to be managed over the TI-SCI protocol.
> 
> Signed-off-by: Dave Gerlach 
> Signed-off-by: Nishanth Menon 
> ---
>  .../devicetree/bindings/soc/ti/sci-pm-domain.txt   | 58 
> ++
>  MAINTAINERS|  1 +
>  2 files changed, 59 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
> 
> diff --git a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt 
> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
> new file mode 100644
> index ..059a5d71d692
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
> @@ -0,0 +1,58 @@
> +Texas Instruments TI-SCI Generic Power Domain
> +-
> +
> +Some TI SoCs contain a system controller (like the PMMC, etc...) that are
> +responsible for control the state of the IPs that are present. Communication
> +between the host processor running an OS and the system controller happens
> +through a protocol known as TI-SCI[1]. This pm domain implementation plugs 
> into
> +the generic pm domain framework and makes use of the TI SCI protocol power on
> +and off each device when needed.
> +
> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
> +
> +PM Domains Nodes
> +
> +The PM domains node represents the global PM domain managed by the PMMC,
> +which in this case is one cell implementation as documented by the generic
> +PM domain bindings in
> +Documentation/devicetree/bindings/power/power_domain.txt.
> +
> +Required Properties:
> +
> +- compatible: should be "ti,sci-pm-domains"
> +- #power-domain-cells: Must be 1 so that an offset can be provided in each
> +device node.
> +- ti,sci: Phandle to the TI SCI device to use for managing the devices

How about just making this a child node of the SCI device?

Rob

[PATCH] iscsi-target: fix spelling mistake "Unsolicitied" -> "Unsolicited"

[Colin King]

From: Colin Ian King 

Trivial fix to spelling mistakes in pr_debug message and comments

Signed-off-by: Colin Ian King 
---
 drivers/target/iscsi/iscsi_target.c   | 2 +-
 drivers/target/iscsi/iscsi_target_login.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target.c 
b/drivers/target/iscsi/iscsi_target.c
index 39b928c..1aaf1f3 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -2982,7 +2982,7 @@ iscsit_build_nopin_rsp(struct iscsi_cmd *cmd, struct 
iscsi_conn *conn,
 
pr_debug("Built NOPIN %s Response ITT: 0x%08x, TTT: 0x%08x,"
" StatSN: 0x%08x, Length %u\n", (nopout_response) ?
-   "Solicitied" : "Unsolicitied", cmd->init_task_tag,
+   "Solicited" : "Unsolicited", cmd->init_task_tag,
cmd->targ_xfer_tag, cmd->stat_sn, cmd->buf_ptr_size);
 }
 EXPORT_SYMBOL(iscsit_build_nopin_rsp);
diff --git a/drivers/target/iscsi/iscsi_target_login.c 
b/drivers/target/iscsi/iscsi_target_login.c
index e1b4caf..78b0d5e 100644
--- a/drivers/target/iscsi/iscsi_target_login.c
+++ b/drivers/target/iscsi/iscsi_target_login.c
@@ -434,7 +434,7 @@ static int iscsi_login_zero_tsih_s2(
 
/*
 * Make MaxRecvDataSegmentLength PAGE_SIZE aligned for
-* Immediate Data + Unsolicitied Data-OUT if necessary..
+* Immediate Data + Unsolicited Data-OUT if necessary..
 */
param = iscsi_find_param_from_key("MaxRecvDataSegmentLength",
  conn->param_list);
@@ -646,7 +646,7 @@ static void iscsi_post_login_start_timers(struct iscsi_conn 
*conn)
 {
struct iscsi_session *sess = conn->sess;
/*
-* FIXME: Unsolicitied NopIN support for ISER
+* FIXME: Unsolicited NopIN support for ISER
 */
if (conn->conn_transport->transport_type == ISCSI_INFINIBAND)
return;
-- 
2.9.3

Re: [PATCH 1/3] Documentation: dt: Add TI-SCI PM Domains

[Rob Herring]

On Fri, Aug 19, 2016 at 06:56:51PM -0500, Nishanth Menon wrote:
> From: Dave Gerlach 
> 
> Add a generic power domain implementation, TI SCI PM Domains, that
> will hook into the genpd framework and allow each PD, which will be
> created one per device, to be managed over the TI-SCI protocol.
> 
> Signed-off-by: Dave Gerlach 
> Signed-off-by: Nishanth Menon 
> ---
>  .../devicetree/bindings/soc/ti/sci-pm-domain.txt   | 58 
> ++
>  MAINTAINERS|  1 +
>  2 files changed, 59 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
> 
> diff --git a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt 
> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
> new file mode 100644
> index ..059a5d71d692
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
> @@ -0,0 +1,58 @@
> +Texas Instruments TI-SCI Generic Power Domain
> +-
> +
> +Some TI SoCs contain a system controller (like the PMMC, etc...) that are
> +responsible for control the state of the IPs that are present. Communication
> +between the host processor running an OS and the system controller happens
> +through a protocol known as TI-SCI[1]. This pm domain implementation plugs 
> into
> +the generic pm domain framework and makes use of the TI SCI protocol power on
> +and off each device when needed.
> +
> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
> +
> +PM Domains Nodes
> +
> +The PM domains node represents the global PM domain managed by the PMMC,
> +which in this case is one cell implementation as documented by the generic
> +PM domain bindings in
> +Documentation/devicetree/bindings/power/power_domain.txt.
> +
> +Required Properties:
> +
> +- compatible: should be "ti,sci-pm-domains"
> +- #power-domain-cells: Must be 1 so that an offset can be provided in each
> +device node.
> +- ti,sci: Phandle to the TI SCI device to use for managing the devices

How about just making this a child node of the SCI device?

Rob

[PATCH] iscsi-target: fix spelling mistake "Unsolicitied" -> "Unsolicited"

[Colin King]

From: Colin Ian King 

Trivial fix to spelling mistakes in pr_debug message and comments

Signed-off-by: Colin Ian King 
---
 drivers/target/iscsi/iscsi_target.c   | 2 +-
 drivers/target/iscsi/iscsi_target_login.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target.c 
b/drivers/target/iscsi/iscsi_target.c
index 39b928c..1aaf1f3 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -2982,7 +2982,7 @@ iscsit_build_nopin_rsp(struct iscsi_cmd *cmd, struct 
iscsi_conn *conn,
 
pr_debug("Built NOPIN %s Response ITT: 0x%08x, TTT: 0x%08x,"
" StatSN: 0x%08x, Length %u\n", (nopout_response) ?
-   "Solicitied" : "Unsolicitied", cmd->init_task_tag,
+   "Solicited" : "Unsolicited", cmd->init_task_tag,
cmd->targ_xfer_tag, cmd->stat_sn, cmd->buf_ptr_size);
 }
 EXPORT_SYMBOL(iscsit_build_nopin_rsp);
diff --git a/drivers/target/iscsi/iscsi_target_login.c 
b/drivers/target/iscsi/iscsi_target_login.c
index e1b4caf..78b0d5e 100644
--- a/drivers/target/iscsi/iscsi_target_login.c
+++ b/drivers/target/iscsi/iscsi_target_login.c
@@ -434,7 +434,7 @@ static int iscsi_login_zero_tsih_s2(
 
/*
 * Make MaxRecvDataSegmentLength PAGE_SIZE aligned for
-* Immediate Data + Unsolicitied Data-OUT if necessary..
+* Immediate Data + Unsolicited Data-OUT if necessary..
 */
param = iscsi_find_param_from_key("MaxRecvDataSegmentLength",
  conn->param_list);
@@ -646,7 +646,7 @@ static void iscsi_post_login_start_timers(struct iscsi_conn 
*conn)
 {
struct iscsi_session *sess = conn->sess;
/*
-* FIXME: Unsolicitied NopIN support for ISER
+* FIXME: Unsolicited NopIN support for ISER
 */
if (conn->conn_transport->transport_type == ISCSI_INFINIBAND)
return;
-- 
2.9.3

[PATCH] net: lantiq_etop: Remove unused 'i' variable

[Paul Burton]

Commit e7f4dc3536a4 ("mdio: Move allocation of interrupts into core")
removed the only use of the 'i' variable from ltq_etop_mdio_init() but
left the variable declaration behind, leading to the following compiler
warning:

  drivers/net/ethernet/lantiq_etop.c: In function 'ltq_etop_mdio_init':
  drivers/net/ethernet/lantiq_etop.c:414:6: warning: unused variable 'i' 
[-Wunused-variable]
int i;
^

Fix this by removing the declaration of the 'i' variable.

Signed-off-by: Paul Burton 

---

 drivers/net/ethernet/lantiq_etop.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/lantiq_etop.c 
b/drivers/net/ethernet/lantiq_etop.c
index dc82b1b..0d2f8e9 100644
--- a/drivers/net/ethernet/lantiq_etop.c
+++ b/drivers/net/ethernet/lantiq_etop.c
@@ -411,7 +411,6 @@ static int
 ltq_etop_mdio_init(struct net_device *dev)
 {
struct ltq_etop_priv *priv = netdev_priv(dev);
-   int i;
int err;
 
priv->mii_bus = mdiobus_alloc();
-- 
2.9.3

[PATCH] net: lantiq_etop: Remove unused 'i' variable

[Paul Burton]

Commit e7f4dc3536a4 ("mdio: Move allocation of interrupts into core")
removed the only use of the 'i' variable from ltq_etop_mdio_init() but
left the variable declaration behind, leading to the following compiler
warning:

  drivers/net/ethernet/lantiq_etop.c: In function 'ltq_etop_mdio_init':
  drivers/net/ethernet/lantiq_etop.c:414:6: warning: unused variable 'i' 
[-Wunused-variable]
int i;
^

Fix this by removing the declaration of the 'i' variable.

Signed-off-by: Paul Burton 

---

 drivers/net/ethernet/lantiq_etop.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/lantiq_etop.c 
b/drivers/net/ethernet/lantiq_etop.c
index dc82b1b..0d2f8e9 100644
--- a/drivers/net/ethernet/lantiq_etop.c
+++ b/drivers/net/ethernet/lantiq_etop.c
@@ -411,7 +411,6 @@ static int
 ltq_etop_mdio_init(struct net_device *dev)
 {
struct ltq_etop_priv *priv = netdev_priv(dev);
-   int i;
int err;
 
priv->mii_bus = mdiobus_alloc();
-- 
2.9.3

Re: [PATCH 3/4] KVM: x86: introduce get_kvmclock_ns

[Roman Kagan]

On Thu, Sep 01, 2016 at 05:26:14PM +0200, Paolo Bonzini wrote:
> Introduce a function that reads the exact nanoseconds value that is
> provided to the guest in kvmclock.  This crystallizes the notion of
> kvmclock as a thin veneer over a stable TSC, that the guest will
> (hopefully) convert with NTP.  In other words, kvmclock is *not* a
> paravirtualized host-to-guest NTP.
> 
> Drop the get_kernel_ns() function, that was used both to get the base
> value of the master clock and to get the current value of kvmclock.
> The former use is replaced by ktime_get_boot_ns(), the latter is
> the purpose of get_kernel_ns().
> 
> This also allows KVM to provide a Hyper-V time reference counter that
> is synchronized with the time that is computed from the TSC page.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  arch/x86/entry/vdso/vclock_gettime.c |  2 +-
>  arch/x86/include/asm/pvclock.h   |  5 ++--
>  arch/x86/kernel/pvclock.c|  2 +-
>  arch/x86/kvm/hyperv.c|  2 +-
>  arch/x86/kvm/x86.c   | 48 
> +++-
>  arch/x86/kvm/x86.h   |  6 +
>  6 files changed, 43 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/x86/entry/vdso/vclock_gettime.c 
> b/arch/x86/entry/vdso/vclock_gettime.c
> index 94d54d0defa7..02223cb4bcfd 100644
> --- a/arch/x86/entry/vdso/vclock_gettime.c
> +++ b/arch/x86/entry/vdso/vclock_gettime.c
> @@ -129,7 +129,7 @@ static notrace cycle_t vread_pvclock(int *mode)
>   return 0;
>   }
>  
> - ret = __pvclock_read_cycles(pvti);
> + ret = __pvclock_read_cycles(pvti, rdtsc_ordered());
>   } while (pvclock_read_retry(pvti, version));
>  
>   /* refer to vread_tsc() comment for rationale */
> diff --git a/arch/x86/include/asm/pvclock.h b/arch/x86/include/asm/pvclock.h
> index d019f0cc80ec..3ad741b84072 100644
> --- a/arch/x86/include/asm/pvclock.h
> +++ b/arch/x86/include/asm/pvclock.h
> @@ -87,9 +87,10 @@ static inline u64 pvclock_scale_delta(u64 delta, u32 
> mul_frac, int shift)
>  }
>  
>  static __always_inline
> -cycle_t __pvclock_read_cycles(const struct pvclock_vcpu_time_info *src)
> +cycle_t __pvclock_read_cycles(const struct pvclock_vcpu_time_info *src,
> +   u64 tsc)
>  {
> - u64 delta = rdtsc_ordered() - src->tsc_timestamp;
> + u64 delta = tsc - src->tsc_timestamp;
>   cycle_t offset = pvclock_scale_delta(delta, src->tsc_to_system_mul,
>src->tsc_shift);
>   return src->system_time + offset;
> diff --git a/arch/x86/kernel/pvclock.c b/arch/x86/kernel/pvclock.c
> index 3599404e3089..5b2cc889ce34 100644
> --- a/arch/x86/kernel/pvclock.c
> +++ b/arch/x86/kernel/pvclock.c
> @@ -80,7 +80,7 @@ cycle_t pvclock_clocksource_read(struct 
> pvclock_vcpu_time_info *src)
>  
>   do {
>   version = pvclock_read_begin(src);
> - ret = __pvclock_read_cycles(src);
> + ret = __pvclock_read_cycles(src, rdtsc_ordered());
>   flags = src->flags;
>   } while (pvclock_read_retry(src, version));
>  

Perhaps adding an argument to __pvclock_read_cycles deserves a separate
patch, to get timekeeping folks' ack on it?

> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 01bd7b7a6866..ed5b77f39ffb 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -386,7 +386,7 @@ static void synic_init(struct kvm_vcpu_hv_synic *synic)
>  
>  static u64 get_time_ref_counter(struct kvm *kvm)
>  {
> - return div_u64(get_kernel_ns() + kvm->arch.kvmclock_offset, 100);
> + return div_u64(get_kvmclock_ns(kvm), 100);

Since this does slightly different calculation than the real hyperv tsc
ref page clock is supposed to, I wonder if we are safe WRT precision
errors leading to occasional monotonicity violations?

>  }
>  
>  static void stimer_mark_pending(struct kvm_vcpu_hv_stimer *stimer,
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index b5853b86b67d..2edcfa054cbe 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1425,7 +1425,7 @@ void kvm_write_tsc(struct kvm_vcpu *vcpu, struct 
> msr_data *msr)
>  
>   raw_spin_lock_irqsave(>arch.tsc_write_lock, flags);
>   offset = kvm_compute_tsc_offset(vcpu, data);
> - ns = get_kernel_ns();
> + ns = ktime_get_boot_ns();
>   elapsed = ns - kvm->arch.last_tsc_nsec;
>  
>   if (vcpu->arch.virtual_tsc_khz) {
> @@ -1716,6 +1716,34 @@ static void kvm_gen_update_masterclock(struct kvm *kvm)
>  #endif
>  }
>  
> +static u64 __get_kvmclock_ns(struct kvm *kvm)
> +{
> + struct kvm_vcpu *vcpu = kvm_get_vcpu(kvm, 0);
> + struct kvm_arch *ka = >arch;
> + s64 ns;
> +
> + if (vcpu->arch.hv_clock.flags & PVCLOCK_TSC_STABLE_BIT) {
> + u64 tsc = kvm_read_l1_tsc(vcpu, rdtsc());
> + ns = __pvclock_read_cycles(>arch.hv_clock, tsc);
> + } else {
> + ns = 

Re: [PATCH 3/4] KVM: x86: introduce get_kvmclock_ns

[Roman Kagan]

On Thu, Sep 01, 2016 at 05:26:14PM +0200, Paolo Bonzini wrote:
> Introduce a function that reads the exact nanoseconds value that is
> provided to the guest in kvmclock.  This crystallizes the notion of
> kvmclock as a thin veneer over a stable TSC, that the guest will
> (hopefully) convert with NTP.  In other words, kvmclock is *not* a
> paravirtualized host-to-guest NTP.
> 
> Drop the get_kernel_ns() function, that was used both to get the base
> value of the master clock and to get the current value of kvmclock.
> The former use is replaced by ktime_get_boot_ns(), the latter is
> the purpose of get_kernel_ns().
> 
> This also allows KVM to provide a Hyper-V time reference counter that
> is synchronized with the time that is computed from the TSC page.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  arch/x86/entry/vdso/vclock_gettime.c |  2 +-
>  arch/x86/include/asm/pvclock.h   |  5 ++--
>  arch/x86/kernel/pvclock.c|  2 +-
>  arch/x86/kvm/hyperv.c|  2 +-
>  arch/x86/kvm/x86.c   | 48 
> +++-
>  arch/x86/kvm/x86.h   |  6 +
>  6 files changed, 43 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/x86/entry/vdso/vclock_gettime.c 
> b/arch/x86/entry/vdso/vclock_gettime.c
> index 94d54d0defa7..02223cb4bcfd 100644
> --- a/arch/x86/entry/vdso/vclock_gettime.c
> +++ b/arch/x86/entry/vdso/vclock_gettime.c
> @@ -129,7 +129,7 @@ static notrace cycle_t vread_pvclock(int *mode)
>   return 0;
>   }
>  
> - ret = __pvclock_read_cycles(pvti);
> + ret = __pvclock_read_cycles(pvti, rdtsc_ordered());
>   } while (pvclock_read_retry(pvti, version));
>  
>   /* refer to vread_tsc() comment for rationale */
> diff --git a/arch/x86/include/asm/pvclock.h b/arch/x86/include/asm/pvclock.h
> index d019f0cc80ec..3ad741b84072 100644
> --- a/arch/x86/include/asm/pvclock.h
> +++ b/arch/x86/include/asm/pvclock.h
> @@ -87,9 +87,10 @@ static inline u64 pvclock_scale_delta(u64 delta, u32 
> mul_frac, int shift)
>  }
>  
>  static __always_inline
> -cycle_t __pvclock_read_cycles(const struct pvclock_vcpu_time_info *src)
> +cycle_t __pvclock_read_cycles(const struct pvclock_vcpu_time_info *src,
> +   u64 tsc)
>  {
> - u64 delta = rdtsc_ordered() - src->tsc_timestamp;
> + u64 delta = tsc - src->tsc_timestamp;
>   cycle_t offset = pvclock_scale_delta(delta, src->tsc_to_system_mul,
>src->tsc_shift);
>   return src->system_time + offset;
> diff --git a/arch/x86/kernel/pvclock.c b/arch/x86/kernel/pvclock.c
> index 3599404e3089..5b2cc889ce34 100644
> --- a/arch/x86/kernel/pvclock.c
> +++ b/arch/x86/kernel/pvclock.c
> @@ -80,7 +80,7 @@ cycle_t pvclock_clocksource_read(struct 
> pvclock_vcpu_time_info *src)
>  
>   do {
>   version = pvclock_read_begin(src);
> - ret = __pvclock_read_cycles(src);
> + ret = __pvclock_read_cycles(src, rdtsc_ordered());
>   flags = src->flags;
>   } while (pvclock_read_retry(src, version));
>  

Perhaps adding an argument to __pvclock_read_cycles deserves a separate
patch, to get timekeeping folks' ack on it?

> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 01bd7b7a6866..ed5b77f39ffb 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -386,7 +386,7 @@ static void synic_init(struct kvm_vcpu_hv_synic *synic)
>  
>  static u64 get_time_ref_counter(struct kvm *kvm)
>  {
> - return div_u64(get_kernel_ns() + kvm->arch.kvmclock_offset, 100);
> + return div_u64(get_kvmclock_ns(kvm), 100);

Since this does slightly different calculation than the real hyperv tsc
ref page clock is supposed to, I wonder if we are safe WRT precision
errors leading to occasional monotonicity violations?

>  }
>  
>  static void stimer_mark_pending(struct kvm_vcpu_hv_stimer *stimer,
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index b5853b86b67d..2edcfa054cbe 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1425,7 +1425,7 @@ void kvm_write_tsc(struct kvm_vcpu *vcpu, struct 
> msr_data *msr)
>  
>   raw_spin_lock_irqsave(>arch.tsc_write_lock, flags);
>   offset = kvm_compute_tsc_offset(vcpu, data);
> - ns = get_kernel_ns();
> + ns = ktime_get_boot_ns();
>   elapsed = ns - kvm->arch.last_tsc_nsec;
>  
>   if (vcpu->arch.virtual_tsc_khz) {
> @@ -1716,6 +1716,34 @@ static void kvm_gen_update_masterclock(struct kvm *kvm)
>  #endif
>  }
>  
> +static u64 __get_kvmclock_ns(struct kvm *kvm)
> +{
> + struct kvm_vcpu *vcpu = kvm_get_vcpu(kvm, 0);
> + struct kvm_arch *ka = >arch;
> + s64 ns;
> +
> + if (vcpu->arch.hv_clock.flags & PVCLOCK_TSC_STABLE_BIT) {
> + u64 tsc = kvm_read_l1_tsc(vcpu, rdtsc());
> + ns = __pvclock_read_cycles(>arch.hv_clock, tsc);
> + } else {
> + ns = ktime_get_boot_ns() + 

Re: Block-level access

[Alex Austin]

My access is almost purely sequential and primarily writing, so read-ahead
doesn't help me. What's problematic with pread/pwrite is the lack of error
channel from media errors.

BSG looks very interesting. I'll look further into that today.

On Thu, Sep 1, 2016 at 5:16 PM, Bart Van Assche
 wrote:
> On 09/01/2016 02:48 PM, Alex Austin wrote:
>>
>> CC'ing linux-scsi and linux-block.
>>
>> Also, please CC me in replies.
>>
>> On Thu, Sep 1, 2016 at 4:43 PM, Alex Austin 
>> wrote:
>>>
>>> Hello,
>>> What is the most performant way to directly interface with an attached
>>> hard
>>> drive? I've so far used read()/write() on /dev/sd_ but I find error
>>> handling
>>> exceedingly difficult, as I don't always even get errors reported, even
>>> if the
>>> open() call includes O_DIRECT. I've also used ioctl(SG_IO), but find that
>>> it's
>>> extremely slow due to the lack of queuing support in the API. Is there a
>>> mid-level API that will get me decent error handling while allowing
>>> command
>>> queuing, or do I just need to make multiple threads all doing separate
>>> SG_IO
>>> ioctls?
>
>
> What the most efficient way is to interface is with a hard drive depends on
> the I/O pattern. Are you aware that buffered I/O performs read-ahead? Have
> you already tried to tune the read-ahead setting (blockdev --getra /
> blockdev --setra)?
>
> BTW, if you need an example of how to queue SG_IO, you are welcome to have a
> look at the fio source code. You will probably be interested in the bsg API.
> See e.g. https://lwn.net/Articles/174469/.
>
> Bart.



-- 
Intelligence is knowing that a tomoato is a fruit; wisdom is knowing
that it doesn't go in a fruit salad; charisma is selling a
tomato-based fruit salad.

Re: Block-level access

[Alex Austin]

My access is almost purely sequential and primarily writing, so read-ahead
doesn't help me. What's problematic with pread/pwrite is the lack of error
channel from media errors.

BSG looks very interesting. I'll look further into that today.

On Thu, Sep 1, 2016 at 5:16 PM, Bart Van Assche
 wrote:
> On 09/01/2016 02:48 PM, Alex Austin wrote:
>>
>> CC'ing linux-scsi and linux-block.
>>
>> Also, please CC me in replies.
>>
>> On Thu, Sep 1, 2016 at 4:43 PM, Alex Austin 
>> wrote:
>>>
>>> Hello,
>>> What is the most performant way to directly interface with an attached
>>> hard
>>> drive? I've so far used read()/write() on /dev/sd_ but I find error
>>> handling
>>> exceedingly difficult, as I don't always even get errors reported, even
>>> if the
>>> open() call includes O_DIRECT. I've also used ioctl(SG_IO), but find that
>>> it's
>>> extremely slow due to the lack of queuing support in the API. Is there a
>>> mid-level API that will get me decent error handling while allowing
>>> command
>>> queuing, or do I just need to make multiple threads all doing separate
>>> SG_IO
>>> ioctls?
>
>
> What the most efficient way is to interface is with a hard drive depends on
> the I/O pattern. Are you aware that buffered I/O performs read-ahead? Have
> you already tried to tune the read-ahead setting (blockdev --getra /
> blockdev --setra)?
>
> BTW, if you need an example of how to queue SG_IO, you are welcome to have a
> look at the fio source code. You will probably be interested in the bsg API.
> See e.g. https://lwn.net/Articles/174469/.
>
> Bart.



-- 
Intelligence is knowing that a tomoato is a fruit; wisdom is knowing
that it doesn't go in a fruit salad; charisma is selling a
tomato-based fruit salad.

[PATCH] net: ti: cpmac: Fix compiler warning due to type confusion

[Paul Burton]

cpmac_start_xmit() used the max() macro on skb->len (an unsigned int)
and ETH_ZLEN (a signed int literal). This led to the following compiler
warning:

  In file included from include/linux/list.h:8:0,
   from include/linux/module.h:9,
   from drivers/net/ethernet/ti/cpmac.c:19:
  drivers/net/ethernet/ti/cpmac.c: In function 'cpmac_start_xmit':
  include/linux/kernel.h:748:17: warning: comparison of distinct pointer
  types lacks a cast
(void) (&_max1 == &_max2);  \
   ^
  drivers/net/ethernet/ti/cpmac.c:560:8: note: in expansion of macro 'max'
len = max(skb->len, ETH_ZLEN);
  ^

On top of this, it assigned the result of the max() macro to a signed
integer whilst all further uses of it result in it being cast to varying
widths of unsigned integer.

Fix this up by using max_t to ensure the comparison is performed as
unsigned integers, and for consistency change the type of the len
variable to unsigned int.

Signed-off-by: Paul Burton 

---

 drivers/net/ethernet/ti/cpmac.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ti/cpmac.c b/drivers/net/ethernet/ti/cpmac.c
index 7eef45e..b1454bd 100644
--- a/drivers/net/ethernet/ti/cpmac.c
+++ b/drivers/net/ethernet/ti/cpmac.c
@@ -547,7 +547,8 @@ fatal_error:
 
 static int cpmac_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
-   int queue, len;
+   int queue;
+   unsigned int len;
struct cpmac_desc *desc;
struct cpmac_priv *priv = netdev_priv(dev);
 
@@ -557,7 +558,7 @@ static int cpmac_start_xmit(struct sk_buff *skb, struct 
net_device *dev)
if (unlikely(skb_padto(skb, ETH_ZLEN)))
return NETDEV_TX_OK;
 
-   len = max(skb->len, ETH_ZLEN);
+   len = max_t(unsigned int, skb->len, ETH_ZLEN);
queue = skb_get_queue_mapping(skb);
netif_stop_subqueue(dev, queue);
 
-- 
2.9.3

Re: [PATCH 2/2] arm64: dts: rockchip: add eMMC's power domain support for rk3399

[Ziyuan Xu]

Hi Ulf,


On 2016年09月02日 18:24, Ulf Hansson wrote:

On 1 September 2016 at 23:50, Doug Anderson  wrote:

>Hi,
>
>On Thu, Sep 1, 2016 at 6:45 AM, Ulf Hansson  wrote:

>>I was reading the discussion regarding this change and browsing the DT
>>documentation around this... Can you guys explain what really goes on
>>here, please.
>>
>>To me, it seems like you are managing one device's resources in one
>>separate genpd. One genpd per device. Is that correct?
>>
>>Perhaps each device actually has its own PM domain and thus it makes
>>sense to assign one genpd per device?

>
>I'm not as familiar with genpd as I should be, so hopefully this makes sense.
>
>...in hardware there is a "pd_emmc" that is the power domain for just
>eMMC.  That will be referenced hooked up via device tree, like:
>
>power-domains = < RK3399_PD_EMMC>;

Yes, I noticed that and this is what puzzles me a bit.


>
>I believe that means that power will automatically be removed whenever
>the device is runtime suspended or suspended.

Well, it depends if the genpd has a subdomain or other devices in it
being runtime resumed.
The genpd will not power off unless all devices within it are runtime
enabled+suspended and that its subdomains are also powered off.

So, in case you only have one device and no subdomains, then your
statement is correct.


Yup, pd_emmc is a individual power domain which is only deployed to eMMC 
on rk3399. It has no subdomains.





>
>If w're not supporting "autosuspend" and nobody is tweaking anything

I guess you mean runtime PM autosuspend? Then why don't you support this?

Wouldn't that allow you to avoid wasting power in runtime when the
device is idle?


pd_emmc manages the sdhci controller, phy and corecfg_* stuff, if we 
support autosuspend in driver, we have to re-init context. I didn't test 
the latency, if it's acceptable, we will apply it.:-)

But it's not a blocker, right?




>manually, then it's possible (I think) that runtime suspend happens at
>exactly the same time as suspend.  ...but my point was that it was

I am not sure I follow you here. You must not rely on that the device
always becomes runtime suspended during system suspend, as there are
no guarantees for this.

Instead that is something you need to take care of in the
subsystem/driver for the device, of course.


>cleaner to actually do it any restoring in the "runtime resume" hooks
>to match what genpd does.  This matches what you say: use runtime PM.

Yes!

Using genpd without deploying runtime PM for the devices doesn't make
much sense, at least to me.


>
>...but it also sounds like it might not be terribly important to
>restore these values since they're a bit silly to begin with.  If
>that's true then I guess we don't need to do anything special here.
>
>
>Did that all make sense (it's entirely possible it didn't since
>somehow my brain still hasn't absorbed all runtime PM and genpd
>concepts)

No worries. I understand this might be a bit tricky, that's why I also
tries to help review related changes.

Kind regards
Uffe

[PATCH] net: ti: cpmac: Fix compiler warning due to type confusion

[Paul Burton]

cpmac_start_xmit() used the max() macro on skb->len (an unsigned int)
and ETH_ZLEN (a signed int literal). This led to the following compiler
warning:

  In file included from include/linux/list.h:8:0,
   from include/linux/module.h:9,
   from drivers/net/ethernet/ti/cpmac.c:19:
  drivers/net/ethernet/ti/cpmac.c: In function 'cpmac_start_xmit':
  include/linux/kernel.h:748:17: warning: comparison of distinct pointer
  types lacks a cast
(void) (&_max1 == &_max2);  \
   ^
  drivers/net/ethernet/ti/cpmac.c:560:8: note: in expansion of macro 'max'
len = max(skb->len, ETH_ZLEN);
  ^

On top of this, it assigned the result of the max() macro to a signed
integer whilst all further uses of it result in it being cast to varying
widths of unsigned integer.

Fix this up by using max_t to ensure the comparison is performed as
unsigned integers, and for consistency change the type of the len
variable to unsigned int.

Signed-off-by: Paul Burton 

---

 drivers/net/ethernet/ti/cpmac.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ti/cpmac.c b/drivers/net/ethernet/ti/cpmac.c
index 7eef45e..b1454bd 100644
--- a/drivers/net/ethernet/ti/cpmac.c
+++ b/drivers/net/ethernet/ti/cpmac.c
@@ -547,7 +547,8 @@ fatal_error:
 
 static int cpmac_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
-   int queue, len;
+   int queue;
+   unsigned int len;
struct cpmac_desc *desc;
struct cpmac_priv *priv = netdev_priv(dev);
 
@@ -557,7 +558,7 @@ static int cpmac_start_xmit(struct sk_buff *skb, struct 
net_device *dev)
if (unlikely(skb_padto(skb, ETH_ZLEN)))
return NETDEV_TX_OK;
 
-   len = max(skb->len, ETH_ZLEN);
+   len = max_t(unsigned int, skb->len, ETH_ZLEN);
queue = skb_get_queue_mapping(skb);
netif_stop_subqueue(dev, queue);
 
-- 
2.9.3

Re: [PATCH 2/2] arm64: dts: rockchip: add eMMC's power domain support for rk3399

[Ziyuan Xu]

Hi Ulf,


On 2016年09月02日 18:24, Ulf Hansson wrote:

On 1 September 2016 at 23:50, Doug Anderson  wrote:

>Hi,
>
>On Thu, Sep 1, 2016 at 6:45 AM, Ulf Hansson  wrote:

>>I was reading the discussion regarding this change and browsing the DT
>>documentation around this... Can you guys explain what really goes on
>>here, please.
>>
>>To me, it seems like you are managing one device's resources in one
>>separate genpd. One genpd per device. Is that correct?
>>
>>Perhaps each device actually has its own PM domain and thus it makes
>>sense to assign one genpd per device?

>
>I'm not as familiar with genpd as I should be, so hopefully this makes sense.
>
>...in hardware there is a "pd_emmc" that is the power domain for just
>eMMC.  That will be referenced hooked up via device tree, like:
>
>power-domains = < RK3399_PD_EMMC>;

Yes, I noticed that and this is what puzzles me a bit.


>
>I believe that means that power will automatically be removed whenever
>the device is runtime suspended or suspended.

Well, it depends if the genpd has a subdomain or other devices in it
being runtime resumed.
The genpd will not power off unless all devices within it are runtime
enabled+suspended and that its subdomains are also powered off.

So, in case you only have one device and no subdomains, then your
statement is correct.


Yup, pd_emmc is a individual power domain which is only deployed to eMMC 
on rk3399. It has no subdomains.





>
>If w're not supporting "autosuspend" and nobody is tweaking anything

I guess you mean runtime PM autosuspend? Then why don't you support this?

Wouldn't that allow you to avoid wasting power in runtime when the
device is idle?


pd_emmc manages the sdhci controller, phy and corecfg_* stuff, if we 
support autosuspend in driver, we have to re-init context. I didn't test 
the latency, if it's acceptable, we will apply it.:-)

But it's not a blocker, right?




>manually, then it's possible (I think) that runtime suspend happens at
>exactly the same time as suspend.  ...but my point was that it was

I am not sure I follow you here. You must not rely on that the device
always becomes runtime suspended during system suspend, as there are
no guarantees for this.

Instead that is something you need to take care of in the
subsystem/driver for the device, of course.


>cleaner to actually do it any restoring in the "runtime resume" hooks
>to match what genpd does.  This matches what you say: use runtime PM.

Yes!

Using genpd without deploying runtime PM for the devices doesn't make
much sense, at least to me.


>
>...but it also sounds like it might not be terribly important to
>restore these values since they're a bit silly to begin with.  If
>that's true then I guess we don't need to do anything special here.
>
>
>Did that all make sense (it's entirely possible it didn't since
>somehow my brain still hasn't absorbed all runtime PM and genpd
>concepts)

No worries. I understand this might be a bit tricky, that's why I also
tries to help review related changes.

Kind regards
Uffe

Re: [PATCH] usb: dwc3: host: inherit dma configuration from parent dev

[Alan Stern]

On Fri, 2 Sep 2016, Felipe Balbi wrote:

> Hi,
> 
> Russell King - ARM Linux  writes:
> > On Fri, Sep 02, 2016 at 12:43:39PM +0200, Arnd Bergmann wrote:
> >> On Thursday, September 1, 2016 5:14:28 PM CEST Leo Li wrote:
> >> > 
> >> > Hi Felipe and Arnd,
> >> > 
> >> > It has been a while since the last response to this discussion, but we
> >> > haven't reached an agreement yet!  Can we get to a conclusion on if it
> >> > is valid to create child platform device for abstraction purpose?  If
> >> > yes, can this child device do DMA by itself?
> >> 
> >> I'd say it's no problem for a driver to create child devices in order
> >> to represent different aspects of a device, but you should not rely on
> >> those devices working when used with the dma-mapping interfaces.
> >
> > That's absolutely right.  Consider the USB model - only the USB host
> > controller can perform DMA, not the USB devices themselves.  All DMA
> > mappings need to be mapped using the USB host controller device struct
> > not the USB device struct.
> >
> > The same _should_ be true everywhere else: the struct device representing
> > the device performing DMA must be the one used to map the transfer.
> 
> How do we fix dwc3 in dual-role, then?
> 
> Peripheral-side dwc3 is easy, we just require a glue-layer to be present
> and use dwc3.ko's parent device (which will be the PCI device or OF
> device). But for host side dwc3, the problem is slightly more complex
> because we're using xhci-plat.ko by just instantiating a xhci-platform
> device so xhci-plat can probe.
> 
> xhci core has no means to know if its own device or the parent of its
> parent should be used for DMA. Any ideas?

In theory, you can store a flag somewhere in the platform device,
something that would tell xhci-hcd that it has to use the parent's
parent for DMA purposes.

I know it would be somewhat of a hack, but ought to work.

Alan Stern

Re: [PATCH] usb: dwc3: host: inherit dma configuration from parent dev

[Alan Stern]

On Fri, 2 Sep 2016, Felipe Balbi wrote:

> Hi,
> 
> Russell King - ARM Linux  writes:
> > On Fri, Sep 02, 2016 at 12:43:39PM +0200, Arnd Bergmann wrote:
> >> On Thursday, September 1, 2016 5:14:28 PM CEST Leo Li wrote:
> >> > 
> >> > Hi Felipe and Arnd,
> >> > 
> >> > It has been a while since the last response to this discussion, but we
> >> > haven't reached an agreement yet!  Can we get to a conclusion on if it
> >> > is valid to create child platform device for abstraction purpose?  If
> >> > yes, can this child device do DMA by itself?
> >> 
> >> I'd say it's no problem for a driver to create child devices in order
> >> to represent different aspects of a device, but you should not rely on
> >> those devices working when used with the dma-mapping interfaces.
> >
> > That's absolutely right.  Consider the USB model - only the USB host
> > controller can perform DMA, not the USB devices themselves.  All DMA
> > mappings need to be mapped using the USB host controller device struct
> > not the USB device struct.
> >
> > The same _should_ be true everywhere else: the struct device representing
> > the device performing DMA must be the one used to map the transfer.
> 
> How do we fix dwc3 in dual-role, then?
> 
> Peripheral-side dwc3 is easy, we just require a glue-layer to be present
> and use dwc3.ko's parent device (which will be the PCI device or OF
> device). But for host side dwc3, the problem is slightly more complex
> because we're using xhci-plat.ko by just instantiating a xhci-platform
> device so xhci-plat can probe.
> 
> xhci core has no means to know if its own device or the parent of its
> parent should be used for DMA. Any ideas?

In theory, you can store a flag somewhere in the platform device,
something that would tell xhci-hcd that it has to use the parent's
parent for DMA purposes.

I know it would be somewhat of a hack, but ought to work.

Alan Stern

Re: [Question] about patch: don't use [delayed_]work_pending()

[Tejun Heo]

On Fri, Sep 02, 2016 at 09:50:07AM -0400, Tejun Heo wrote:
> Hello,
> 
> On Fri, Sep 02, 2016 at 09:17:04AM +0800, qiaozhou wrote:
> > > > I don't know whether it's meaningful to still check pending work here, 
> > > > or
> > > > it's not suggested to use pm_qos_update_request in this early boot up 
> > > > phase.
> > > > Could you help to share some opinions? (I can fix this issue by adding 
> > > > the
> > > > current qos value directly instead of default value, though.)
> > > Hmmm... but I suppose this is super-early in the boot.  Would it make
> > > sense to have a static variable (e.g. bool clk_fully_initailized) to
> > > gate the cancel_delayed_sync() call?
> >
> > You're right that it's indeed super-early stage. But currently we can't
> > control the gate of can_delayed_work_sync, since it's inside
> > pm_qos_update_request. Out of our control. We can choose to not call
> > pm_qos_update_request to avoid this issue, and use pm_qos_add_request
> > alternatively. Good to have it.
> 
> Ah sorry, didn't understand that the offending cancel_sync call is in
> the generic part.  Hmm... but yeah, we should still be able to take
> the same approach.  I'll see what's the right thing to gate the
> operation there.

Does the following patch work?

Subject: power: avoid calling cancel_delayed_work_sync() during early boot

of_clk_init() ends up calling into pm_qos_update_request() very early
during boot where irq is expected to stay disabled.
pm_qos_update_request() uses cancel_delayed_work_sync() which
correctly assumes that irq is enabled on invocation and
unconditionally disables and re-enables it.

Gate cancel_delayed_work_sync() invocation with kevented_up() to avoid
enabling irq unexpectedly during early boot.

Signed-off-by: Tejun Heo 
Reported-by: Qiao Zhou 
Link: http://lkml.kernel.org/r/d2501c4c-8e7b-bea3-1b01-000b36b5d...@asrmicro.com
---
 kernel/power/qos.c |   11 ++-
  1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/kernel/power/qos.c b/kernel/power/qos.c
index 97b0df7..168ff44 100644
--- a/kernel/power/qos.c
+++ b/kernel/power/qos.c
@@ -482,7 +482,16 @@ void pm_qos_update_request(struct pm_qos_request *req,
return;
}
 
-   cancel_delayed_work_sync(>work);
+   /*
+* This function may be called very early during boot, for example,
+* from of_clk_init(), where irq needs to stay disabled.
+* cancel_delayed_work_sync() assumes that irq is enabled on
+* invocation and re-enables it on return.  Avoid calling it until
+* workqueue is initialized.
+*/
+   if (keventd_up())
+   cancel_delayed_work_sync(>work);
+
__pm_qos_update_request(req, new_value);
 }
 EXPORT_SYMBOL_GPL(pm_qos_update_request);

Re: [Question] about patch: don't use [delayed_]work_pending()

[Tejun Heo]

On Fri, Sep 02, 2016 at 09:50:07AM -0400, Tejun Heo wrote:
> Hello,
> 
> On Fri, Sep 02, 2016 at 09:17:04AM +0800, qiaozhou wrote:
> > > > I don't know whether it's meaningful to still check pending work here, 
> > > > or
> > > > it's not suggested to use pm_qos_update_request in this early boot up 
> > > > phase.
> > > > Could you help to share some opinions? (I can fix this issue by adding 
> > > > the
> > > > current qos value directly instead of default value, though.)
> > > Hmmm... but I suppose this is super-early in the boot.  Would it make
> > > sense to have a static variable (e.g. bool clk_fully_initailized) to
> > > gate the cancel_delayed_sync() call?
> >
> > You're right that it's indeed super-early stage. But currently we can't
> > control the gate of can_delayed_work_sync, since it's inside
> > pm_qos_update_request. Out of our control. We can choose to not call
> > pm_qos_update_request to avoid this issue, and use pm_qos_add_request
> > alternatively. Good to have it.
> 
> Ah sorry, didn't understand that the offending cancel_sync call is in
> the generic part.  Hmm... but yeah, we should still be able to take
> the same approach.  I'll see what's the right thing to gate the
> operation there.

Does the following patch work?

Subject: power: avoid calling cancel_delayed_work_sync() during early boot

of_clk_init() ends up calling into pm_qos_update_request() very early
during boot where irq is expected to stay disabled.
pm_qos_update_request() uses cancel_delayed_work_sync() which
correctly assumes that irq is enabled on invocation and
unconditionally disables and re-enables it.

Gate cancel_delayed_work_sync() invocation with kevented_up() to avoid
enabling irq unexpectedly during early boot.

Signed-off-by: Tejun Heo 
Reported-by: Qiao Zhou 
Link: http://lkml.kernel.org/r/d2501c4c-8e7b-bea3-1b01-000b36b5d...@asrmicro.com
---
 kernel/power/qos.c |   11 ++-
  1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/kernel/power/qos.c b/kernel/power/qos.c
index 97b0df7..168ff44 100644
--- a/kernel/power/qos.c
+++ b/kernel/power/qos.c
@@ -482,7 +482,16 @@ void pm_qos_update_request(struct pm_qos_request *req,
return;
}
 
-   cancel_delayed_work_sync(>work);
+   /*
+* This function may be called very early during boot, for example,
+* from of_clk_init(), where irq needs to stay disabled.
+* cancel_delayed_work_sync() assumes that irq is enabled on
+* invocation and re-enables it on return.  Avoid calling it until
+* workqueue is initialized.
+*/
+   if (keventd_up())
+   cancel_delayed_work_sync(>work);
+
__pm_qos_update_request(req, new_value);
 }
 EXPORT_SYMBOL_GPL(pm_qos_update_request);

[PATCH] MIPS: dec: Avoid la pseudo-instruction in delay slots

[Paul Burton]

When expanding the la or dla pseudo-instruction in a delay slot the GNU
assembler will complain should the pseudo-instruction expand to multiple
actual instructions, since only the first of them will be in the delay
slot leading to the pseudo-instruction being only partially executed if
the branch is taken. Use of PTR_LA in the dec int-handler.S leads to
such warnings:

  arch/mips/dec/int-handler.S: Assembler messages:
  arch/mips/dec/int-handler.S:149: Warning: macro instruction expanded into 
multiple instructions in a branch delay slot
  arch/mips/dec/int-handler.S:198: Warning: macro instruction expanded into 
multiple instructions in a branch delay slot

Avoid this by placing nops in the delay slots of the affected branches,
leading to the PTR_LA macros being placed after the branches & their
delay slots. Although the nop isn't strictly needed, it's an
insignificant cost & satisfies the assembler easily with more
readable code than the possible alternative of manually expanding the
la/dla pseudo-instructions & placing the appropriate first instruction
into the delay slots.

Signed-off-by: Paul Burton 
---

 arch/mips/dec/int-handler.S | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/mips/dec/int-handler.S b/arch/mips/dec/int-handler.S
index d7b9918..54ddca1 100644
--- a/arch/mips/dec/int-handler.S
+++ b/arch/mips/dec/int-handler.S
@@ -137,16 +137,16 @@
and t0,t1   # isolate allowed ones
 
beqzt0,spurious
-
 #ifdef CONFIG_32BIT
 andt2,t0
bnezt2,fpu  # handle FPU immediately
 #endif
+nop
 
/*
 * Find irq with highest priority
 */
-PTR_LA t1,cpu_mask_nr_tbl
+   PTR_LA  t1,cpu_mask_nr_tbl
 1: lw  t2,(t1)
nop
and t2,t0
@@ -191,11 +191,12 @@
 1: and t0,t1   # mask out allowed ones
 
beqzt0,spurious
+nop
 
/*
 * Find irq with highest priority
 */
-PTR_LA t1,asic_mask_nr_tbl
+   PTR_LA  t1,asic_mask_nr_tbl
 2: lw  t2,(t1)
nop
and t2,t0
-- 
2.9.3

Re: [PATCH v15 04/13] task_isolation: add initial support

[Chris Metcalf]

On 9/1/2016 6:06 AM, Peter Zijlstra wrote:

On Tue, Aug 30, 2016 at 11:32:16AM -0400, Chris Metcalf wrote:

On 8/30/2016 3:58 AM, Peter Zijlstra wrote:

What !? I really don't get this, what are you waiting for? Why is
rescheduling making things better.

We need to wait for the last dyntick to fire before we can return to
userspace.  There are plenty of options as to what we can do in the
meanwhile.

Why not keep your _TIF_TASK_ISOLATION_FOO flag set and re-enter the
loop?

I really don't see how setting TIF_NEED_RESCHED is helping anything.


Yes, I think I addressed that in an earlier reply to Frederic; and you're right,
I don't think TIF_NEED_RESCHED or schedule() are the way to go.

https://lkml.kernel.org/g/107bd666-dbcf-7fa5-ff9c-f79358899...@mellanox.com

Any thoughts on the question of "just re-enter the loop" vs. schedule_timeout()?

--
Chris Metcalf, Mellanox Technologies
http://www.mellanox.com

[PATCH] MIPS: dec: Avoid la pseudo-instruction in delay slots

[Paul Burton]

When expanding the la or dla pseudo-instruction in a delay slot the GNU
assembler will complain should the pseudo-instruction expand to multiple
actual instructions, since only the first of them will be in the delay
slot leading to the pseudo-instruction being only partially executed if
the branch is taken. Use of PTR_LA in the dec int-handler.S leads to
such warnings:

  arch/mips/dec/int-handler.S: Assembler messages:
  arch/mips/dec/int-handler.S:149: Warning: macro instruction expanded into 
multiple instructions in a branch delay slot
  arch/mips/dec/int-handler.S:198: Warning: macro instruction expanded into 
multiple instructions in a branch delay slot

Avoid this by placing nops in the delay slots of the affected branches,
leading to the PTR_LA macros being placed after the branches & their
delay slots. Although the nop isn't strictly needed, it's an
insignificant cost & satisfies the assembler easily with more
readable code than the possible alternative of manually expanding the
la/dla pseudo-instructions & placing the appropriate first instruction
into the delay slots.

Signed-off-by: Paul Burton 
---

 arch/mips/dec/int-handler.S | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/mips/dec/int-handler.S b/arch/mips/dec/int-handler.S
index d7b9918..54ddca1 100644
--- a/arch/mips/dec/int-handler.S
+++ b/arch/mips/dec/int-handler.S
@@ -137,16 +137,16 @@
and t0,t1   # isolate allowed ones
 
beqzt0,spurious
-
 #ifdef CONFIG_32BIT
 andt2,t0
bnezt2,fpu  # handle FPU immediately
 #endif
+nop
 
/*
 * Find irq with highest priority
 */
-PTR_LA t1,cpu_mask_nr_tbl
+   PTR_LA  t1,cpu_mask_nr_tbl
 1: lw  t2,(t1)
nop
and t2,t0
@@ -191,11 +191,12 @@
 1: and t0,t1   # mask out allowed ones
 
beqzt0,spurious
+nop
 
/*
 * Find irq with highest priority
 */
-PTR_LA t1,asic_mask_nr_tbl
+   PTR_LA  t1,asic_mask_nr_tbl
 2: lw  t2,(t1)
nop
and t2,t0
-- 
2.9.3

Re: [PATCH v15 04/13] task_isolation: add initial support

[Chris Metcalf]

On 9/1/2016 6:06 AM, Peter Zijlstra wrote:

On Tue, Aug 30, 2016 at 11:32:16AM -0400, Chris Metcalf wrote:

On 8/30/2016 3:58 AM, Peter Zijlstra wrote:

What !? I really don't get this, what are you waiting for? Why is
rescheduling making things better.

We need to wait for the last dyntick to fire before we can return to
userspace.  There are plenty of options as to what we can do in the
meanwhile.

Why not keep your _TIF_TASK_ISOLATION_FOO flag set and re-enter the
loop?

I really don't see how setting TIF_NEED_RESCHED is helping anything.


Yes, I think I addressed that in an earlier reply to Frederic; and you're right,
I don't think TIF_NEED_RESCHED or schedule() are the way to go.

https://lkml.kernel.org/g/107bd666-dbcf-7fa5-ff9c-f79358899...@mellanox.com

Any thoughts on the question of "just re-enter the loop" vs. schedule_timeout()?

--
Chris Metcalf, Mellanox Technologies
http://www.mellanox.com

[PATCH] MIPS: netlogic: Fix assembler warning from smpboot.S

[Paul Burton]

The netlogic platform can be built for either MIPS32 or MIPS64, and when
built for MIPS32 (as by nlm_xlr_defconfig) the use of the dla
pseudo-instruction leads to warnings such as the following from recent
versions of the GNU assembler:

  arch/mips/netlogic/common/smpboot.S: Assembler messages:
  arch/mips/netlogic/common/smpboot.S:62: Warning: dla used to load 32-bit 
register; recommend using la instead
  arch/mips/netlogic/common/smpboot.S:63: Warning: dla used to load 32-bit 
register; recommend using la instead

Avoid these warnings by using the PTR_LA macro to make use of the
appropriate la or dla pseudo-instruction for the build.

Signed-off-by: Paul Burton 
---

 arch/mips/netlogic/common/smpboot.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/mips/netlogic/common/smpboot.S 
b/arch/mips/netlogic/common/smpboot.S
index f0cc4c9..509c1a7 100644
--- a/arch/mips/netlogic/common/smpboot.S
+++ b/arch/mips/netlogic/common/smpboot.S
@@ -59,8 +59,8 @@ NESTED(xlp_boot_core0_siblings, PT_SIZE, sp)
sync
/* find the location to which nlm_boot_siblings was relocated */
li  t0, CKSEG1ADDR(RESET_VEC_PHYS)
-   dla t1, nlm_reset_entry
-   dla t2, nlm_boot_siblings
+   PTR_LA  t1, nlm_reset_entry
+   PTR_LA  t2, nlm_boot_siblings
dsubu   t2, t1
daddu   t2, t0
/* call it */
-- 
2.9.3

[PATCH] MIPS: netlogic: Fix assembler warning from smpboot.S

[Paul Burton]

The netlogic platform can be built for either MIPS32 or MIPS64, and when
built for MIPS32 (as by nlm_xlr_defconfig) the use of the dla
pseudo-instruction leads to warnings such as the following from recent
versions of the GNU assembler:

  arch/mips/netlogic/common/smpboot.S: Assembler messages:
  arch/mips/netlogic/common/smpboot.S:62: Warning: dla used to load 32-bit 
register; recommend using la instead
  arch/mips/netlogic/common/smpboot.S:63: Warning: dla used to load 32-bit 
register; recommend using la instead

Avoid these warnings by using the PTR_LA macro to make use of the
appropriate la or dla pseudo-instruction for the build.

Signed-off-by: Paul Burton 
---

 arch/mips/netlogic/common/smpboot.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/mips/netlogic/common/smpboot.S 
b/arch/mips/netlogic/common/smpboot.S
index f0cc4c9..509c1a7 100644
--- a/arch/mips/netlogic/common/smpboot.S
+++ b/arch/mips/netlogic/common/smpboot.S
@@ -59,8 +59,8 @@ NESTED(xlp_boot_core0_siblings, PT_SIZE, sp)
sync
/* find the location to which nlm_boot_siblings was relocated */
li  t0, CKSEG1ADDR(RESET_VEC_PHYS)
-   dla t1, nlm_reset_entry
-   dla t2, nlm_boot_siblings
+   PTR_LA  t1, nlm_reset_entry
+   PTR_LA  t2, nlm_boot_siblings
dsubu   t2, t1
daddu   t2, t0
/* call it */
-- 
2.9.3

Re: [PATCH 1/6] irqchip: mips-gic: Add context saving for MIPS_REMOTEPROC

[Matt Redfearn]

Hi Marc,

Thanks for the review!


On 02/09/16 11:54, Marc Zyngier wrote:

Hi Matt,

On 02/09/16 10:59, Matt Redfearn wrote:

The MIPS remote processor driver allows non-Linux firmware to take
control of and execute on one of the systems VPEs. If that VPE is
brought back under Linux, it is necessary to ensure that all GIC
interrupts are routed and masked as Linux expects them, as the firmware
can have done anything it likes with the GIC configuration (hopefully
just for that VPEs local interrupt sources, but allow for shared
external interrupts as well).

The configuration of shared and local CPU interrupts is maintained and
updated every time a change is made. When a CPU is brought online, the
saved configuration is restored.

These functions will also be useful for restoring GIC context after a
suspend to RAM.

Signed-off-by: Matt Redfearn 
---

  drivers/irqchip/irq-mips-gic.c | 185 +++--
  1 file changed, 178 insertions(+), 7 deletions(-)

diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c
index 83f498393a7f..5ba1fe1468ce 100644
--- a/drivers/irqchip/irq-mips-gic.c
+++ b/drivers/irqchip/irq-mips-gic.c
@@ -8,6 +8,7 @@
   */
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -56,6 +57,47 @@ static unsigned int timer_cpu_pin;
  static struct irq_chip gic_level_irq_controller, gic_edge_irq_controller;
  DECLARE_BITMAP(ipi_resrv, GIC_MAX_INTRS);
  
+#if defined(CONFIG_MIPS_RPROC)

+#define CONTEXT_SAVING
+#endif
+
+#ifdef CONTEXT_SAVING

This looks really cumbersome. Why not make everything depend on
CONFIG_MIPS_RPROC instead?


The idea was that the context saving is useful for more than just the 
remote processor driver, for example suspending the state to ram would 
need this. The plan was to have additional things turn on context saving 
here, but with just he one user I agree it looks a bit odd. I could 
perhaps just make everything selected by CONFIG_MIPS_RPROC to start off 
with and then change that in the future when additional users get merged.





+static struct {
+   unsigned mask:  GIC_NUM_LOCAL_INTRS;

nit/personal taste: can't you make this a normal type instead of a
bitfield? Considering that GIC_NUM_LOCAL_INTRS is hardcoded to 7, I'd
rather see a u8... or even an unsigned long if you have to use bitmap
operations on it.


Sure, no problem.




+} gic_local_state[NR_CPUS];

This looks like this really should be a percpu variable.


Yes, it probably can be.




+
+#define gic_save_local_rmask(vpe, i)   (gic_local_state[vpe].mask &= i)
+#define gic_save_local_smask(vpe, i)   (gic_local_state[vpe].mask |= i)
+
+static struct {
+   unsigned vpe:   8;
+   unsigned pin:   4;
+
+   unsigned polarity:  1;
+   unsigned trigger:   1;
+   unsigned dual_edge: 1;
+   unsigned mask:  1;
+} gic_shared_state[GIC_MAX_INTRS];
+
+#define gic_save_shared_vpe(i, v)  gic_shared_state[i].vpe = v
+#define gic_save_shared_pin(i, p)  gic_shared_state[i].pin = p
+#define gic_save_shared_polarity(i, p) gic_shared_state[i].polarity = p
+#define gic_save_shared_trigger(i, t)  gic_shared_state[i].trigger = t
+#define gic_save_shared_dual_edge(i, d)gic_shared_state[i].dual_edge = 
d
+#define gic_save_shared_mask(i, m) gic_shared_state[i].mask = m

Why don't you make these static functions? The compiler will inline them
nicely, and that will save you fixing them (they all miss proper
bracketing of arguments).


Sure, makes sense.




+
+#else
+#define gic_save_local_rmask(vpe, i)
+#define gic_save_local_smask(vpe, i)
+
+#define gic_save_shared_vpe(i, v)
+#define gic_save_shared_pin(i, p)
+#define gic_save_shared_polarity(i, p)
+#define gic_save_shared_trigger(i, t)
+#define gic_save_shared_dual_edge(i, d)
+#define gic_save_shared_mask(i, m)

Please make those a "do { } while(0)" construct, so that the trailing
semi-colon is properly swallowed.


OK.




+#endif /* CONTEXT_SAVING */
+
  static void __gic_irq_dispatch(void);
  
  static inline u32 gic_read32(unsigned int reg)

@@ -105,52 +147,94 @@ static inline void gic_update_bits(unsigned int reg, 
unsigned long mask,
gic_write(reg, regval);
  }
  
-static inline void gic_reset_mask(unsigned int intr)

+static inline void gic_write_reset_mask(unsigned int intr)
  {
gic_write(GIC_REG(SHARED, GIC_SH_RMASK) + GIC_INTR_OFS(intr),
  1ul << GIC_INTR_BIT(intr));
  }
  
-static inline void gic_set_mask(unsigned int intr)

+static inline void gic_reset_mask(unsigned int intr)
+{
+   gic_save_shared_mask(intr, 0);
+   gic_write_reset_mask(intr);
+}
+
+static inline void gic_write_set_mask(unsigned int intr)
  {
gic_write(GIC_REG(SHARED, GIC_SH_SMASK) + GIC_INTR_OFS(intr),
  1ul << GIC_INTR_BIT(intr));
  }
  
-static inline void gic_set_polarity(unsigned int intr, unsigned int pol)

+static inline void 

Re: [PATCH 1/6] irqchip: mips-gic: Add context saving for MIPS_REMOTEPROC

[Matt Redfearn]

Hi Marc,

Thanks for the review!


On 02/09/16 11:54, Marc Zyngier wrote:

Hi Matt,

On 02/09/16 10:59, Matt Redfearn wrote:

The MIPS remote processor driver allows non-Linux firmware to take
control of and execute on one of the systems VPEs. If that VPE is
brought back under Linux, it is necessary to ensure that all GIC
interrupts are routed and masked as Linux expects them, as the firmware
can have done anything it likes with the GIC configuration (hopefully
just for that VPEs local interrupt sources, but allow for shared
external interrupts as well).

The configuration of shared and local CPU interrupts is maintained and
updated every time a change is made. When a CPU is brought online, the
saved configuration is restored.

These functions will also be useful for restoring GIC context after a
suspend to RAM.

Signed-off-by: Matt Redfearn 
---

  drivers/irqchip/irq-mips-gic.c | 185 +++--
  1 file changed, 178 insertions(+), 7 deletions(-)

diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c
index 83f498393a7f..5ba1fe1468ce 100644
--- a/drivers/irqchip/irq-mips-gic.c
+++ b/drivers/irqchip/irq-mips-gic.c
@@ -8,6 +8,7 @@
   */
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -56,6 +57,47 @@ static unsigned int timer_cpu_pin;
  static struct irq_chip gic_level_irq_controller, gic_edge_irq_controller;
  DECLARE_BITMAP(ipi_resrv, GIC_MAX_INTRS);
  
+#if defined(CONFIG_MIPS_RPROC)

+#define CONTEXT_SAVING
+#endif
+
+#ifdef CONTEXT_SAVING

This looks really cumbersome. Why not make everything depend on
CONFIG_MIPS_RPROC instead?


The idea was that the context saving is useful for more than just the 
remote processor driver, for example suspending the state to ram would 
need this. The plan was to have additional things turn on context saving 
here, but with just he one user I agree it looks a bit odd. I could 
perhaps just make everything selected by CONFIG_MIPS_RPROC to start off 
with and then change that in the future when additional users get merged.





+static struct {
+   unsigned mask:  GIC_NUM_LOCAL_INTRS;

nit/personal taste: can't you make this a normal type instead of a
bitfield? Considering that GIC_NUM_LOCAL_INTRS is hardcoded to 7, I'd
rather see a u8... or even an unsigned long if you have to use bitmap
operations on it.


Sure, no problem.




+} gic_local_state[NR_CPUS];

This looks like this really should be a percpu variable.


Yes, it probably can be.




+
+#define gic_save_local_rmask(vpe, i)   (gic_local_state[vpe].mask &= i)
+#define gic_save_local_smask(vpe, i)   (gic_local_state[vpe].mask |= i)
+
+static struct {
+   unsigned vpe:   8;
+   unsigned pin:   4;
+
+   unsigned polarity:  1;
+   unsigned trigger:   1;
+   unsigned dual_edge: 1;
+   unsigned mask:  1;
+} gic_shared_state[GIC_MAX_INTRS];
+
+#define gic_save_shared_vpe(i, v)  gic_shared_state[i].vpe = v
+#define gic_save_shared_pin(i, p)  gic_shared_state[i].pin = p
+#define gic_save_shared_polarity(i, p) gic_shared_state[i].polarity = p
+#define gic_save_shared_trigger(i, t)  gic_shared_state[i].trigger = t
+#define gic_save_shared_dual_edge(i, d)gic_shared_state[i].dual_edge = 
d
+#define gic_save_shared_mask(i, m) gic_shared_state[i].mask = m

Why don't you make these static functions? The compiler will inline them
nicely, and that will save you fixing them (they all miss proper
bracketing of arguments).


Sure, makes sense.




+
+#else
+#define gic_save_local_rmask(vpe, i)
+#define gic_save_local_smask(vpe, i)
+
+#define gic_save_shared_vpe(i, v)
+#define gic_save_shared_pin(i, p)
+#define gic_save_shared_polarity(i, p)
+#define gic_save_shared_trigger(i, t)
+#define gic_save_shared_dual_edge(i, d)
+#define gic_save_shared_mask(i, m)

Please make those a "do { } while(0)" construct, so that the trailing
semi-colon is properly swallowed.


OK.




+#endif /* CONTEXT_SAVING */
+
  static void __gic_irq_dispatch(void);
  
  static inline u32 gic_read32(unsigned int reg)

@@ -105,52 +147,94 @@ static inline void gic_update_bits(unsigned int reg, 
unsigned long mask,
gic_write(reg, regval);
  }
  
-static inline void gic_reset_mask(unsigned int intr)

+static inline void gic_write_reset_mask(unsigned int intr)
  {
gic_write(GIC_REG(SHARED, GIC_SH_RMASK) + GIC_INTR_OFS(intr),
  1ul << GIC_INTR_BIT(intr));
  }
  
-static inline void gic_set_mask(unsigned int intr)

+static inline void gic_reset_mask(unsigned int intr)
+{
+   gic_save_shared_mask(intr, 0);
+   gic_write_reset_mask(intr);
+}
+
+static inline void gic_write_set_mask(unsigned int intr)
  {
gic_write(GIC_REG(SHARED, GIC_SH_SMASK) + GIC_INTR_OFS(intr),
  1ul << GIC_INTR_BIT(intr));
  }
  
-static inline void gic_set_polarity(unsigned int intr, unsigned int pol)

+static inline void gic_set_mask(unsigned int intr)
+{
+   

[PATCH] MIPS: Fix detection of unsupported highmem with cache aliases

[Paul Burton]

The paging_init() function contains code which detects that highmem is
in use but unsupported due to dcache aliasing. However this code was
ineffective because it was being run before the caches are probed,
meaning that cpu_has_dc_aliases would always evaluate to false (unless a
platform overrides it to a compile-time constant) and the detection of
the unsupported case is never triggered. The kernel would then go on to
attempt to use highmem & either hit coherency issues or trigger the
BUG_ON in flush_kernel_dcache_page().

Fix this by running paging_init() later than cpu_cache_init(), such that
the cpu_has_dc_aliases macro will evaluate correctly & the unsupported
highmem case will be detected successfully.

This then leads to a formerly hidden issue in that
mem_init_free_highmem() will attempt to free all highmem pages, even
though we're avoiding use of them & don't have valid page structs for
them. This leads to an invalid pointer dereference & a TLB exception.
Avoid this by skipping the loop in mem_init_free_highmem() if
cpu_has_dc_aliases evaluates true.

Signed-off-by: Paul Burton 
---

 arch/mips/kernel/setup.c | 2 +-
 arch/mips/mm/init.c  | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index ef408a0..d840f02 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -757,7 +757,6 @@ static void __init arch_mem_init(char **cmdline_p)
device_tree_init();
sparse_init();
plat_swiotlb_setup();
-   paging_init();
 
dma_contiguous_reserve(PFN_PHYS(max_low_pfn));
/* Tell bootmem about cma reserved memblock section */
@@ -870,6 +869,7 @@ void __init setup_arch(char **cmdline_p)
prefill_possible_map();
 
cpu_cache_init();
+   paging_init();
 }
 
 unsigned long kernelsp[NR_CPUS];
diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
index 9b58eb5..ea6d8b6 100644
--- a/arch/mips/mm/init.c
+++ b/arch/mips/mm/init.c
@@ -441,6 +441,9 @@ static inline void mem_init_free_highmem(void)
 #ifdef CONFIG_HIGHMEM
unsigned long tmp;
 
+   if (cpu_has_dc_aliases)
+   return;
+
for (tmp = highstart_pfn; tmp < highend_pfn; tmp++) {
struct page *page = pfn_to_page(tmp);
 
-- 
2.9.3

[PATCH] MIPS: Fix detection of unsupported highmem with cache aliases

[Paul Burton]

The paging_init() function contains code which detects that highmem is
in use but unsupported due to dcache aliasing. However this code was
ineffective because it was being run before the caches are probed,
meaning that cpu_has_dc_aliases would always evaluate to false (unless a
platform overrides it to a compile-time constant) and the detection of
the unsupported case is never triggered. The kernel would then go on to
attempt to use highmem & either hit coherency issues or trigger the
BUG_ON in flush_kernel_dcache_page().

Fix this by running paging_init() later than cpu_cache_init(), such that
the cpu_has_dc_aliases macro will evaluate correctly & the unsupported
highmem case will be detected successfully.

This then leads to a formerly hidden issue in that
mem_init_free_highmem() will attempt to free all highmem pages, even
though we're avoiding use of them & don't have valid page structs for
them. This leads to an invalid pointer dereference & a TLB exception.
Avoid this by skipping the loop in mem_init_free_highmem() if
cpu_has_dc_aliases evaluates true.

Signed-off-by: Paul Burton 
---

 arch/mips/kernel/setup.c | 2 +-
 arch/mips/mm/init.c  | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index ef408a0..d840f02 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -757,7 +757,6 @@ static void __init arch_mem_init(char **cmdline_p)
device_tree_init();
sparse_init();
plat_swiotlb_setup();
-   paging_init();
 
dma_contiguous_reserve(PFN_PHYS(max_low_pfn));
/* Tell bootmem about cma reserved memblock section */
@@ -870,6 +869,7 @@ void __init setup_arch(char **cmdline_p)
prefill_possible_map();
 
cpu_cache_init();
+   paging_init();
 }
 
 unsigned long kernelsp[NR_CPUS];
diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
index 9b58eb5..ea6d8b6 100644
--- a/arch/mips/mm/init.c
+++ b/arch/mips/mm/init.c
@@ -441,6 +441,9 @@ static inline void mem_init_free_highmem(void)
 #ifdef CONFIG_HIGHMEM
unsigned long tmp;
 
+   if (cpu_has_dc_aliases)
+   return;
+
for (tmp = highstart_pfn; tmp < highend_pfn; tmp++) {
struct page *page = pfn_to_page(tmp);
 
-- 
2.9.3

Re: [PATCH 4/4] kvm: x86: export TSC offset to user-space

[Steven Rostedt]

On Fri, 2 Sep 2016 09:43:01 -0400
Stefan Hajnoczi  wrote:

> Can TSC offset changes occur at runtime?
> 
> One example is vcpu hotplug where the tracing tool would need to fetch
> the new vcpu's TSC offset after tracing has already started.
> 
> Another example is if QEMU or the guest change the TSC offset while
> running.  If the tracing tool doesn't notice this then trace events will have
> incorrect timestamps.

I believe there are tracepoints for these events. They would obviously
need to be enabled for the tracer to catch them.

I would also recommend that they go into their own instance to make
sure other events do not overwrite them.

-- Steve

Re: [PATCH v4 1/3] nvmem: rockchip-efuse: update compatible strings for Rockchip efuse

[Rob Herring]

On Mon, Aug 29, 2016 at 02:50:08AM -0700, Finley Xiao wrote:
> Rk3399-efuse is organized as 32bits by 32 one-time programmable electrical
> fuses. The efuse of earlier SoCs are organized as 32bits by 8 one-time
> programmable electrical fuses with random access interface.
> 
> Add different device tree compatible string for different SoCs to be able
> to differentiate between the two. The old binding is of course preserved,
> though deprecated.
> 
> Signed-off-by: Finley Xiao 
> Reviewed-by: Heiko Stuebner 
> ---
>  Documentation/devicetree/bindings/nvmem/rockchip-efuse.txt | 13 +++--
>  1 file changed, 11 insertions(+), 2 deletions(-)

Acked-by: Rob Herring 

Re: [PATCH 4/4] kvm: x86: export TSC offset to user-space

[Steven Rostedt]

On Fri, 2 Sep 2016 09:43:01 -0400
Stefan Hajnoczi  wrote:

> Can TSC offset changes occur at runtime?
> 
> One example is vcpu hotplug where the tracing tool would need to fetch
> the new vcpu's TSC offset after tracing has already started.
> 
> Another example is if QEMU or the guest change the TSC offset while
> running.  If the tracing tool doesn't notice this then trace events will have
> incorrect timestamps.

I believe there are tracepoints for these events. They would obviously
need to be enabled for the tracer to catch them.

I would also recommend that they go into their own instance to make
sure other events do not overwrite them.

-- Steve

Re: [PATCH v4 1/3] nvmem: rockchip-efuse: update compatible strings for Rockchip efuse

[Rob Herring]

On Mon, Aug 29, 2016 at 02:50:08AM -0700, Finley Xiao wrote:
> Rk3399-efuse is organized as 32bits by 32 one-time programmable electrical
> fuses. The efuse of earlier SoCs are organized as 32bits by 8 one-time
> programmable electrical fuses with random access interface.
> 
> Add different device tree compatible string for different SoCs to be able
> to differentiate between the two. The old binding is of course preserved,
> though deprecated.
> 
> Signed-off-by: Finley Xiao 
> Reviewed-by: Heiko Stuebner 
> ---
>  Documentation/devicetree/bindings/nvmem/rockchip-efuse.txt | 13 +++--
>  1 file changed, 11 insertions(+), 2 deletions(-)

Acked-by: Rob Herring 

Re: [PART2 PATCH v7 00/12] iommu/AMD: Introduce IOMMU AVIC support

[Suravee Suthikulpanit]

Thanks All. Please let me know if you need anything else from my side.

Suravee

On 9/2/16 21:05, Joerg Roedel wrote:

Hi Paolo,

On Fri, Sep 02, 2016 at 12:46:28PM +0200, Paolo Bonzini wrote:

Joerg, if there's no other issues, could you apply the first 9 patches
to a branch based on 4.8-rc1 or similar, so that I can pull it into the
KVM tree?


Sure, I was actually waiting for your Acked-By to put all the
patches into the IOMMU tree, but this will work too :) I'll let you know
when I pushed the branch.


Regards,

Joerg

Re: [PART2 PATCH v7 00/12] iommu/AMD: Introduce IOMMU AVIC support

[Suravee Suthikulpanit]

Thanks All. Please let me know if you need anything else from my side.

Suravee

On 9/2/16 21:05, Joerg Roedel wrote:

Hi Paolo,

On Fri, Sep 02, 2016 at 12:46:28PM +0200, Paolo Bonzini wrote:

Joerg, if there's no other issues, could you apply the first 9 patches
to a branch based on 4.8-rc1 or similar, so that I can pull it into the
KVM tree?


Sure, I was actually waiting for your Acked-By to put all the
patches into the IOMMU tree, but this will work too :) I'll let you know
when I pushed the branch.


Regards,

Joerg

Re: [PATCH 1/4] Documentation: mmc: sdhci-of-arasan: Add clk_syscon as an optional one

[Rob Herring]

On Mon, Aug 29, 2016 at 04:02:56PM +0800, Shawn Lin wrote:
> We introduced soc-ctl-syscon to do several things, for instance, update
> baseclk or update clkmul, etc. In odrder to access this physical block,
> we need to explicitly enable its clock. Currently we don't control this
> clock as we always add a CLK_IGNORE_UNUSED flag for it to indicate that
> we will not gate it even if not referenced. This is not a correct way since
> it is a clock parenting from clk_ahb which is used by sdhci-of-arasan now.
> Without enabling clk_ahb, the flag don't guarantee we could access
> soc-ctl-syscon. Moreover, we can't find a reason not to gate clk_syscon
> once we remove/power-down emmc controller. So let's add clk_syscon and
> enable/disable it explicitly when needed.
> 
> Signed-off-by: Shawn Lin 
> 
> ---
> 
>  Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt 
> b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> index 3404afa..b04eb02 100644
> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> @@ -33,6 +33,9 @@ Optional Properties:
>- clock-output-names: If specified, this will be the name of the card clock
>  which will be exposed by this device.  Required if #clock-cells is
>  specified.
> +  - clock-names: From clock bindings: Although we treat clock-names as 
> required
> +property, there is still one, "clk_syscon", should be optional as it 
> depends
> +on whether we need to control soc-ctl-syscon or not.

No. This doesn't look right to me. The syscon is a separate block 
and the clock for it belongs in the syscon node itself. Probably there 
needs to be some sort of ref counting in the syscon so it can do 
runtime-pm.

Rob

Re: [PATCH 1/4] Documentation: mmc: sdhci-of-arasan: Add clk_syscon as an optional one

[Rob Herring]

On Mon, Aug 29, 2016 at 04:02:56PM +0800, Shawn Lin wrote:
> We introduced soc-ctl-syscon to do several things, for instance, update
> baseclk or update clkmul, etc. In odrder to access this physical block,
> we need to explicitly enable its clock. Currently we don't control this
> clock as we always add a CLK_IGNORE_UNUSED flag for it to indicate that
> we will not gate it even if not referenced. This is not a correct way since
> it is a clock parenting from clk_ahb which is used by sdhci-of-arasan now.
> Without enabling clk_ahb, the flag don't guarantee we could access
> soc-ctl-syscon. Moreover, we can't find a reason not to gate clk_syscon
> once we remove/power-down emmc controller. So let's add clk_syscon and
> enable/disable it explicitly when needed.
> 
> Signed-off-by: Shawn Lin 
> 
> ---
> 
>  Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt 
> b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> index 3404afa..b04eb02 100644
> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> @@ -33,6 +33,9 @@ Optional Properties:
>- clock-output-names: If specified, this will be the name of the card clock
>  which will be exposed by this device.  Required if #clock-cells is
>  specified.
> +  - clock-names: From clock bindings: Although we treat clock-names as 
> required
> +property, there is still one, "clk_syscon", should be optional as it 
> depends
> +on whether we need to control soc-ctl-syscon or not.

No. This doesn't look right to me. The syscon is a separate block 
and the clock for it belongs in the syscon node itself. Probably there 
needs to be some sort of ref counting in the syscon so it can do 
runtime-pm.

Rob

Re: [PATCH v3 03/22] usb: ulpi: Support device discovery via device properties

[Heikki Krogerus]

Hi,

On Wed, Aug 31, 2016 at 05:40:17PM -0700, Stephen Boyd wrote:
> @@ -174,14 +219,37 @@ static int ulpi_register(struct device *dev, struct 
> ulpi *ulpi)
>   ulpi->id.product = ulpi_read(ulpi, ULPI_PRODUCT_ID_LOW);
>   ulpi->id.product |= ulpi_read(ulpi, ULPI_PRODUCT_ID_HIGH) << 8;
>  
> + return 0;
> +}
> +
> +static int ulpi_register(struct device *dev, struct ulpi *ulpi)
> +{
> + int ret;
> +
>   ulpi->dev.parent = dev;
>   ulpi->dev.bus = _bus;
>   ulpi->dev.type = _dev_type;
>   dev_set_name(>dev, "%s.ulpi", dev_name(dev));
>  
> + if (IS_ENABLED(CONFIG_OF)) {

I don't think you need to check that in this case.

> + ret = ulpi_of_register(ulpi);
> + if (ret)
> + return ret;
> + }
> +
>   ACPI_COMPANION_SET(>dev, ACPI_COMPANION(dev));

ACPI_COMPANION_SET will overwrite the primary fwnode unconditionally,
so just to play it safe, do this before you call ulpi_of_register().

> - request_module("ulpi:v%04xp%04x", ulpi->id.vendor, ulpi->id.product);
> + ret = ulpi_read_id(ulpi);
> + /*
> +  * Ignore failure in case of DT node because the device may
> +  * not be powered up yet but we can still match by compatible
> +  */
> + if (ret && !ulpi->dev.of_node)
> + return ret;
> +
> + if (of_device_request_module(>dev))
> + request_module("ulpi:v%04xp%04x", ulpi->id.vendor,
> +ulpi->id.product);

I don't think this works in all cases. If of_device_request_module()
fails and we don't have the id.vendor/product set, we should not
register the device. It also looks a bit messy.

How about just using of_device_request_module() call as fallback in
ulpi_read_id() and moving also request_module() call there:

diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index 30ea770..667246c 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -205,14 +205,14 @@ static int ulpi_read_id(struct ulpi *ulpi)
/* Test the interface */
ret = ulpi_write(ulpi, ULPI_SCRATCH, 0xaa);
if (ret < 0)
-   return ret;
+   goto err;
 
ret = ulpi_read(ulpi, ULPI_SCRATCH);
if (ret < 0)
-   return ret;
+   goto err;
 
if (ret != 0xaa)
-   return -ENODEV;
+   goto err;
 
ulpi->id.vendor = ulpi_read(ulpi, ULPI_VENDOR_ID_LOW);
ulpi->id.vendor |= ulpi_read(ulpi, ULPI_VENDOR_ID_HIGH) << 8;
@@ -220,7 +220,11 @@ static int ulpi_read_id(struct ulpi *ulpi)
ulpi->id.product = ulpi_read(ulpi, ULPI_PRODUCT_ID_LOW);
ulpi->id.product |= ulpi_read(ulpi, ULPI_PRODUCT_ID_HIGH) << 8;
 
+   request_module("ulpi:v%04xp%04x", ulpi->id.vendor, ulpi->id.product);
+
return 0;
+err:
+   return of_device_request_module(>dev);
 }
 
 static int ulpi_register(struct device *dev, struct ulpi *ulpi)
@@ -232,25 +236,15 @@ static int ulpi_register(struct device *dev, struct ulpi 
*ulpi)
ulpi->dev.type = _dev_type;
dev_set_name(>dev, "%s.ulpi", dev_name(dev));
 
-   if (IS_ENABLED(CONFIG_OF)) {
-   ret = ulpi_of_register(ulpi);
-   if (ret)
-   return ret;
-   }
-
ACPI_COMPANION_SET(>dev, ACPI_COMPANION(dev));
 
-   ret = ulpi_read_id(ulpi);
-   /*
-* Ignore failure in case of DT node because the device may
-* not be powered up yet but we can still match by compatible
-*/
-   if (ret && !ulpi->dev.of_node)
+   ret = ulpi_of_register(ulpi);
+   if (ret)
return ret;
 
-   if (of_device_request_module(>dev))
-   request_module("ulpi:v%04xp%04x", ulpi->id.vendor,
-  ulpi->id.product);
+   ret = ulpi_read_id(ulpi);
+   if (ret)
+   return ret;
 
ret = device_register(>dev);
if (ret)


Cheers,

-- 
heikki

Re: [PATCH] usb: dwc3: host: inherit dma configuration from parent dev

[Felipe Balbi]

Hi,

Felipe Balbi  writes:
> Hi,
>
> Russell King - ARM Linux  writes:
>> On Fri, Sep 02, 2016 at 12:43:39PM +0200, Arnd Bergmann wrote:
>>> On Thursday, September 1, 2016 5:14:28 PM CEST Leo Li wrote:
>>> > 
>>> > Hi Felipe and Arnd,
>>> > 
>>> > It has been a while since the last response to this discussion, but we
>>> > haven't reached an agreement yet!  Can we get to a conclusion on if it
>>> > is valid to create child platform device for abstraction purpose?  If
>>> > yes, can this child device do DMA by itself?
>>> 
>>> I'd say it's no problem for a driver to create child devices in order
>>> to represent different aspects of a device, but you should not rely on
>>> those devices working when used with the dma-mapping interfaces.
>>
>> That's absolutely right.  Consider the USB model - only the USB host
>> controller can perform DMA, not the USB devices themselves.  All DMA
>> mappings need to be mapped using the USB host controller device struct
>> not the USB device struct.
>>
>> The same _should_ be true everywhere else: the struct device representing
>> the device performing DMA must be the one used to map the transfer.
>
> How do we fix dwc3 in dual-role, then?
>
> Peripheral-side dwc3 is easy, we just require a glue-layer to be present
> and use dwc3.ko's parent device (which will be the PCI device or OF
> device). But for host side dwc3, the problem is slightly more complex
> because we're using xhci-plat.ko by just instantiating a xhci-platform
> device so xhci-plat can probe.
>
> xhci core has no means to know if its own device or the parent of its
> parent should be used for DMA. Any ideas?

another thing to consider is that dwc3 only works on omap because DT
defaults to 32-bit DMA mask for anything described in DT that doesn't
provide dma-ranges. Isn't that somewhat odd as well?

Based on your reply, Russell, dwc3-omap should be the DMA device, but
dwc3 works just as well because of the whole 32-bit default.

-- 
balbi

Re: [PATCH v3 03/22] usb: ulpi: Support device discovery via device properties

[Heikki Krogerus]

Hi,

On Wed, Aug 31, 2016 at 05:40:17PM -0700, Stephen Boyd wrote:
> @@ -174,14 +219,37 @@ static int ulpi_register(struct device *dev, struct 
> ulpi *ulpi)
>   ulpi->id.product = ulpi_read(ulpi, ULPI_PRODUCT_ID_LOW);
>   ulpi->id.product |= ulpi_read(ulpi, ULPI_PRODUCT_ID_HIGH) << 8;
>  
> + return 0;
> +}
> +
> +static int ulpi_register(struct device *dev, struct ulpi *ulpi)
> +{
> + int ret;
> +
>   ulpi->dev.parent = dev;
>   ulpi->dev.bus = _bus;
>   ulpi->dev.type = _dev_type;
>   dev_set_name(>dev, "%s.ulpi", dev_name(dev));
>  
> + if (IS_ENABLED(CONFIG_OF)) {

I don't think you need to check that in this case.

> + ret = ulpi_of_register(ulpi);
> + if (ret)
> + return ret;
> + }
> +
>   ACPI_COMPANION_SET(>dev, ACPI_COMPANION(dev));

ACPI_COMPANION_SET will overwrite the primary fwnode unconditionally,
so just to play it safe, do this before you call ulpi_of_register().

> - request_module("ulpi:v%04xp%04x", ulpi->id.vendor, ulpi->id.product);
> + ret = ulpi_read_id(ulpi);
> + /*
> +  * Ignore failure in case of DT node because the device may
> +  * not be powered up yet but we can still match by compatible
> +  */
> + if (ret && !ulpi->dev.of_node)
> + return ret;
> +
> + if (of_device_request_module(>dev))
> + request_module("ulpi:v%04xp%04x", ulpi->id.vendor,
> +ulpi->id.product);

I don't think this works in all cases. If of_device_request_module()
fails and we don't have the id.vendor/product set, we should not
register the device. It also looks a bit messy.

How about just using of_device_request_module() call as fallback in
ulpi_read_id() and moving also request_module() call there:

diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index 30ea770..667246c 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -205,14 +205,14 @@ static int ulpi_read_id(struct ulpi *ulpi)
/* Test the interface */
ret = ulpi_write(ulpi, ULPI_SCRATCH, 0xaa);
if (ret < 0)
-   return ret;
+   goto err;
 
ret = ulpi_read(ulpi, ULPI_SCRATCH);
if (ret < 0)
-   return ret;
+   goto err;
 
if (ret != 0xaa)
-   return -ENODEV;
+   goto err;
 
ulpi->id.vendor = ulpi_read(ulpi, ULPI_VENDOR_ID_LOW);
ulpi->id.vendor |= ulpi_read(ulpi, ULPI_VENDOR_ID_HIGH) << 8;
@@ -220,7 +220,11 @@ static int ulpi_read_id(struct ulpi *ulpi)
ulpi->id.product = ulpi_read(ulpi, ULPI_PRODUCT_ID_LOW);
ulpi->id.product |= ulpi_read(ulpi, ULPI_PRODUCT_ID_HIGH) << 8;
 
+   request_module("ulpi:v%04xp%04x", ulpi->id.vendor, ulpi->id.product);
+
return 0;
+err:
+   return of_device_request_module(>dev);
 }
 
 static int ulpi_register(struct device *dev, struct ulpi *ulpi)
@@ -232,25 +236,15 @@ static int ulpi_register(struct device *dev, struct ulpi 
*ulpi)
ulpi->dev.type = _dev_type;
dev_set_name(>dev, "%s.ulpi", dev_name(dev));
 
-   if (IS_ENABLED(CONFIG_OF)) {
-   ret = ulpi_of_register(ulpi);
-   if (ret)
-   return ret;
-   }
-
ACPI_COMPANION_SET(>dev, ACPI_COMPANION(dev));
 
-   ret = ulpi_read_id(ulpi);
-   /*
-* Ignore failure in case of DT node because the device may
-* not be powered up yet but we can still match by compatible
-*/
-   if (ret && !ulpi->dev.of_node)
+   ret = ulpi_of_register(ulpi);
+   if (ret)
return ret;
 
-   if (of_device_request_module(>dev))
-   request_module("ulpi:v%04xp%04x", ulpi->id.vendor,
-  ulpi->id.product);
+   ret = ulpi_read_id(ulpi);
+   if (ret)
+   return ret;
 
ret = device_register(>dev);
if (ret)


Cheers,

-- 
heikki

Re: [PATCH] usb: dwc3: host: inherit dma configuration from parent dev

[Felipe Balbi]

Hi,

Felipe Balbi  writes:
> Hi,
>
> Russell King - ARM Linux  writes:
>> On Fri, Sep 02, 2016 at 12:43:39PM +0200, Arnd Bergmann wrote:
>>> On Thursday, September 1, 2016 5:14:28 PM CEST Leo Li wrote:
>>> > 
>>> > Hi Felipe and Arnd,
>>> > 
>>> > It has been a while since the last response to this discussion, but we
>>> > haven't reached an agreement yet!  Can we get to a conclusion on if it
>>> > is valid to create child platform device for abstraction purpose?  If
>>> > yes, can this child device do DMA by itself?
>>> 
>>> I'd say it's no problem for a driver to create child devices in order
>>> to represent different aspects of a device, but you should not rely on
>>> those devices working when used with the dma-mapping interfaces.
>>
>> That's absolutely right.  Consider the USB model - only the USB host
>> controller can perform DMA, not the USB devices themselves.  All DMA
>> mappings need to be mapped using the USB host controller device struct
>> not the USB device struct.
>>
>> The same _should_ be true everywhere else: the struct device representing
>> the device performing DMA must be the one used to map the transfer.
>
> How do we fix dwc3 in dual-role, then?
>
> Peripheral-side dwc3 is easy, we just require a glue-layer to be present
> and use dwc3.ko's parent device (which will be the PCI device or OF
> device). But for host side dwc3, the problem is slightly more complex
> because we're using xhci-plat.ko by just instantiating a xhci-platform
> device so xhci-plat can probe.
>
> xhci core has no means to know if its own device or the parent of its
> parent should be used for DMA. Any ideas?

another thing to consider is that dwc3 only works on omap because DT
defaults to 32-bit DMA mask for anything described in DT that doesn't
provide dma-ranges. Isn't that somewhat odd as well?

Based on your reply, Russell, dwc3-omap should be the DMA device, but
dwc3 works just as well because of the whole 32-bit default.

-- 
balbi

Re: [PATCH 3/4] KVM: x86: introduce get_kvmclock_ns

[Paolo Bonzini]

On 02/09/2016 15:52, Roman Kagan wrote:
> On Thu, Sep 01, 2016 at 05:26:14PM +0200, Paolo Bonzini wrote:
>> Introduce a function that reads the exact nanoseconds value that is
>> provided to the guest in kvmclock.  This crystallizes the notion of
>> kvmclock as a thin veneer over a stable TSC, that the guest will
>> (hopefully) convert with NTP.  In other words, kvmclock is *not* a
>> paravirtualized host-to-guest NTP.
>>
>> Drop the get_kernel_ns() function, that was used both to get the base
>> value of the master clock and to get the current value of kvmclock.
>> The former use is replaced by ktime_get_boot_ns(), the latter is
>> the purpose of get_kernel_ns().
>>
>> This also allows KVM to provide a Hyper-V time reference counter that
>> is synchronized with the time that is computed from the TSC page.
>>
>> Signed-off-by: Paolo Bonzini 
>> ---
>>  arch/x86/entry/vdso/vclock_gettime.c |  2 +-
>>  arch/x86/include/asm/pvclock.h   |  5 ++--
>>  arch/x86/kernel/pvclock.c|  2 +-
>>  arch/x86/kvm/hyperv.c|  2 +-
>>  arch/x86/kvm/x86.c   | 48 
>> +++-
>>  arch/x86/kvm/x86.h   |  6 +
>>  6 files changed, 43 insertions(+), 22 deletions(-)
>>
>> diff --git a/arch/x86/entry/vdso/vclock_gettime.c 
>> b/arch/x86/entry/vdso/vclock_gettime.c
>> index 94d54d0defa7..02223cb4bcfd 100644
>> --- a/arch/x86/entry/vdso/vclock_gettime.c
>> +++ b/arch/x86/entry/vdso/vclock_gettime.c
>> @@ -129,7 +129,7 @@ static notrace cycle_t vread_pvclock(int *mode)
>>  return 0;
>>  }
>>  
>> -ret = __pvclock_read_cycles(pvti);
>> +ret = __pvclock_read_cycles(pvti, rdtsc_ordered());
>>  } while (pvclock_read_retry(pvti, version));
>>  
>>  /* refer to vread_tsc() comment for rationale */
>> diff --git a/arch/x86/include/asm/pvclock.h b/arch/x86/include/asm/pvclock.h
>> index d019f0cc80ec..3ad741b84072 100644
>> --- a/arch/x86/include/asm/pvclock.h
>> +++ b/arch/x86/include/asm/pvclock.h
>> @@ -87,9 +87,10 @@ static inline u64 pvclock_scale_delta(u64 delta, u32 
>> mul_frac, int shift)
>>  }
>>  
>>  static __always_inline
>> -cycle_t __pvclock_read_cycles(const struct pvclock_vcpu_time_info *src)
>> +cycle_t __pvclock_read_cycles(const struct pvclock_vcpu_time_info *src,
>> +  u64 tsc)
>>  {
>> -u64 delta = rdtsc_ordered() - src->tsc_timestamp;
>> +u64 delta = tsc - src->tsc_timestamp;
>>  cycle_t offset = pvclock_scale_delta(delta, src->tsc_to_system_mul,
>>   src->tsc_shift);
>>  return src->system_time + offset;
>> diff --git a/arch/x86/kernel/pvclock.c b/arch/x86/kernel/pvclock.c
>> index 3599404e3089..5b2cc889ce34 100644
>> --- a/arch/x86/kernel/pvclock.c
>> +++ b/arch/x86/kernel/pvclock.c
>> @@ -80,7 +80,7 @@ cycle_t pvclock_clocksource_read(struct 
>> pvclock_vcpu_time_info *src)
>>  
>>  do {
>>  version = pvclock_read_begin(src);
>> -ret = __pvclock_read_cycles(src);
>> +ret = __pvclock_read_cycles(src, rdtsc_ordered());
>>  flags = src->flags;
>>  } while (pvclock_read_retry(src, version));
>>  
> 
> Perhaps adding an argument to __pvclock_read_cycles deserves a separate
> patch, to get timekeeping folks' ack on it?
> 
>> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
>> index 01bd7b7a6866..ed5b77f39ffb 100644
>> --- a/arch/x86/kvm/hyperv.c
>> +++ b/arch/x86/kvm/hyperv.c
>> @@ -386,7 +386,7 @@ static void synic_init(struct kvm_vcpu_hv_synic *synic)
>>  
>>  static u64 get_time_ref_counter(struct kvm *kvm)
>>  {
>> -return div_u64(get_kernel_ns() + kvm->arch.kvmclock_offset, 100);
>> +return div_u64(get_kvmclock_ns(kvm), 100);
> 
> Since this does slightly different calculation than the real hyperv tsc
> ref page clock is supposed to, I wonder if we are safe WRT precision
> errors leading to occasional monotonicity violations?

The Hyper-V scale is

 tsc_to_system_mul * 2^(32+tsc_shift) / 100

and the only source of error could be from doing here

 (tsc * tsc_to_system_mul >> (32-tsc_shift)) / 100

vs

 tsc * ((tsc_to_system_mul >> (32-tsc_shift)) / 100))
   ^^
 this is scale / 2^64

in the TSC ref page clock.  If my reasoning is correct the error will be
at most 100 units of the scale value, which is a relative error of
around 1 parts per 2^49.

Likewise for the offset, the improvement from

(tsc - base_tsc) * tsc_to_system_mul >> (32-tsc_shift)
 + base_system_time

vs. using a single offset as in the TSC ref page is one nanosecond---and
the ref page only has a resolution of 100 ns.


Paolo

>>  }
>>  
>>  static void stimer_mark_pending(struct kvm_vcpu_hv_stimer *stimer,
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index b5853b86b67d..2edcfa054cbe 100644
>> --- a/arch/x86/kvm/x86.c

Re: [PATCH 3/4] KVM: x86: introduce get_kvmclock_ns

[Paolo Bonzini]

On 02/09/2016 15:52, Roman Kagan wrote:
> On Thu, Sep 01, 2016 at 05:26:14PM +0200, Paolo Bonzini wrote:
>> Introduce a function that reads the exact nanoseconds value that is
>> provided to the guest in kvmclock.  This crystallizes the notion of
>> kvmclock as a thin veneer over a stable TSC, that the guest will
>> (hopefully) convert with NTP.  In other words, kvmclock is *not* a
>> paravirtualized host-to-guest NTP.
>>
>> Drop the get_kernel_ns() function, that was used both to get the base
>> value of the master clock and to get the current value of kvmclock.
>> The former use is replaced by ktime_get_boot_ns(), the latter is
>> the purpose of get_kernel_ns().
>>
>> This also allows KVM to provide a Hyper-V time reference counter that
>> is synchronized with the time that is computed from the TSC page.
>>
>> Signed-off-by: Paolo Bonzini 
>> ---
>>  arch/x86/entry/vdso/vclock_gettime.c |  2 +-
>>  arch/x86/include/asm/pvclock.h   |  5 ++--
>>  arch/x86/kernel/pvclock.c|  2 +-
>>  arch/x86/kvm/hyperv.c|  2 +-
>>  arch/x86/kvm/x86.c   | 48 
>> +++-
>>  arch/x86/kvm/x86.h   |  6 +
>>  6 files changed, 43 insertions(+), 22 deletions(-)
>>
>> diff --git a/arch/x86/entry/vdso/vclock_gettime.c 
>> b/arch/x86/entry/vdso/vclock_gettime.c
>> index 94d54d0defa7..02223cb4bcfd 100644
>> --- a/arch/x86/entry/vdso/vclock_gettime.c
>> +++ b/arch/x86/entry/vdso/vclock_gettime.c
>> @@ -129,7 +129,7 @@ static notrace cycle_t vread_pvclock(int *mode)
>>  return 0;
>>  }
>>  
>> -ret = __pvclock_read_cycles(pvti);
>> +ret = __pvclock_read_cycles(pvti, rdtsc_ordered());
>>  } while (pvclock_read_retry(pvti, version));
>>  
>>  /* refer to vread_tsc() comment for rationale */
>> diff --git a/arch/x86/include/asm/pvclock.h b/arch/x86/include/asm/pvclock.h
>> index d019f0cc80ec..3ad741b84072 100644
>> --- a/arch/x86/include/asm/pvclock.h
>> +++ b/arch/x86/include/asm/pvclock.h
>> @@ -87,9 +87,10 @@ static inline u64 pvclock_scale_delta(u64 delta, u32 
>> mul_frac, int shift)
>>  }
>>  
>>  static __always_inline
>> -cycle_t __pvclock_read_cycles(const struct pvclock_vcpu_time_info *src)
>> +cycle_t __pvclock_read_cycles(const struct pvclock_vcpu_time_info *src,
>> +  u64 tsc)
>>  {
>> -u64 delta = rdtsc_ordered() - src->tsc_timestamp;
>> +u64 delta = tsc - src->tsc_timestamp;
>>  cycle_t offset = pvclock_scale_delta(delta, src->tsc_to_system_mul,
>>   src->tsc_shift);
>>  return src->system_time + offset;
>> diff --git a/arch/x86/kernel/pvclock.c b/arch/x86/kernel/pvclock.c
>> index 3599404e3089..5b2cc889ce34 100644
>> --- a/arch/x86/kernel/pvclock.c
>> +++ b/arch/x86/kernel/pvclock.c
>> @@ -80,7 +80,7 @@ cycle_t pvclock_clocksource_read(struct 
>> pvclock_vcpu_time_info *src)
>>  
>>  do {
>>  version = pvclock_read_begin(src);
>> -ret = __pvclock_read_cycles(src);
>> +ret = __pvclock_read_cycles(src, rdtsc_ordered());
>>  flags = src->flags;
>>  } while (pvclock_read_retry(src, version));
>>  
> 
> Perhaps adding an argument to __pvclock_read_cycles deserves a separate
> patch, to get timekeeping folks' ack on it?
> 
>> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
>> index 01bd7b7a6866..ed5b77f39ffb 100644
>> --- a/arch/x86/kvm/hyperv.c
>> +++ b/arch/x86/kvm/hyperv.c
>> @@ -386,7 +386,7 @@ static void synic_init(struct kvm_vcpu_hv_synic *synic)
>>  
>>  static u64 get_time_ref_counter(struct kvm *kvm)
>>  {
>> -return div_u64(get_kernel_ns() + kvm->arch.kvmclock_offset, 100);
>> +return div_u64(get_kvmclock_ns(kvm), 100);
> 
> Since this does slightly different calculation than the real hyperv tsc
> ref page clock is supposed to, I wonder if we are safe WRT precision
> errors leading to occasional monotonicity violations?

The Hyper-V scale is

 tsc_to_system_mul * 2^(32+tsc_shift) / 100

and the only source of error could be from doing here

 (tsc * tsc_to_system_mul >> (32-tsc_shift)) / 100

vs

 tsc * ((tsc_to_system_mul >> (32-tsc_shift)) / 100))
   ^^
 this is scale / 2^64

in the TSC ref page clock.  If my reasoning is correct the error will be
at most 100 units of the scale value, which is a relative error of
around 1 parts per 2^49.

Likewise for the offset, the improvement from

(tsc - base_tsc) * tsc_to_system_mul >> (32-tsc_shift)
 + base_system_time

vs. using a single offset as in the TSC ref page is one nanosecond---and
the ref page only has a resolution of 100 ns.


Paolo

>>  }
>>  
>>  static void stimer_mark_pending(struct kvm_vcpu_hv_stimer *stimer,
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index b5853b86b67d..2edcfa054cbe 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ 

[PATCH] ASoC: fsl_esai: fix spelling mistake "Transmition" -> "Transmission"

[Colin King]

From: Colin Ian King 

Trivial fix to spelling mistakes in dev_dbg messages

Signed-off-by: Colin Ian King 
---
 sound/soc/fsl/fsl_esai.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c
index 26a90e1..e927955 100644
--- a/sound/soc/fsl/fsl_esai.c
+++ b/sound/soc/fsl/fsl_esai.c
@@ -77,19 +77,19 @@ static irqreturn_t esai_isr(int irq, void *devid)
regmap_read(esai_priv->regmap, REG_ESAI_ESR, );
 
if (esr & ESAI_ESR_TINIT_MASK)
-   dev_dbg(>dev, "isr: Transmition Initialized\n");
+   dev_dbg(>dev, "isr: Transmission Initialized\n");
 
if (esr & ESAI_ESR_RFF_MASK)
dev_warn(>dev, "isr: Receiving overrun\n");
 
if (esr & ESAI_ESR_TFE_MASK)
-   dev_warn(>dev, "isr: Transmition underrun\n");
+   dev_warn(>dev, "isr: Transmission underrun\n");
 
if (esr & ESAI_ESR_TLS_MASK)
dev_dbg(>dev, "isr: Just transmitted the last slot\n");
 
if (esr & ESAI_ESR_TDE_MASK)
-   dev_dbg(>dev, "isr: Transmition data exception\n");
+   dev_dbg(>dev, "isr: Transmission data exception\n");
 
if (esr & ESAI_ESR_TED_MASK)
dev_dbg(>dev, "isr: Transmitting even slots\n");
-- 
2.9.3

[PATCH] ASoC: fsl_esai: fix spelling mistake "Transmition" -> "Transmission"

[Colin King]

From: Colin Ian King 

Trivial fix to spelling mistakes in dev_dbg messages

Signed-off-by: Colin Ian King 
---
 sound/soc/fsl/fsl_esai.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c
index 26a90e1..e927955 100644
--- a/sound/soc/fsl/fsl_esai.c
+++ b/sound/soc/fsl/fsl_esai.c
@@ -77,19 +77,19 @@ static irqreturn_t esai_isr(int irq, void *devid)
regmap_read(esai_priv->regmap, REG_ESAI_ESR, );
 
if (esr & ESAI_ESR_TINIT_MASK)
-   dev_dbg(>dev, "isr: Transmition Initialized\n");
+   dev_dbg(>dev, "isr: Transmission Initialized\n");
 
if (esr & ESAI_ESR_RFF_MASK)
dev_warn(>dev, "isr: Receiving overrun\n");
 
if (esr & ESAI_ESR_TFE_MASK)
-   dev_warn(>dev, "isr: Transmition underrun\n");
+   dev_warn(>dev, "isr: Transmission underrun\n");
 
if (esr & ESAI_ESR_TLS_MASK)
dev_dbg(>dev, "isr: Just transmitted the last slot\n");
 
if (esr & ESAI_ESR_TDE_MASK)
-   dev_dbg(>dev, "isr: Transmition data exception\n");
+   dev_dbg(>dev, "isr: Transmission data exception\n");
 
if (esr & ESAI_ESR_TED_MASK)
dev_dbg(>dev, "isr: Transmitting even slots\n");
-- 
2.9.3

Re: [RFC PATCH v2 05/20] x86: Add the Secure Memory Encryption cpu feature

[Borislav Petkov]

On Mon, Aug 22, 2016 at 05:36:22PM -0500, Tom Lendacky wrote:
> Update the cpu features to include identifying and reporting on the
> Secure Memory Encryption feature.
> 
> Signed-off-by: Tom Lendacky 
> ---
>  arch/x86/include/asm/cpufeature.h|7 +--
>  arch/x86/include/asm/cpufeatures.h   |5 -
>  arch/x86/include/asm/disabled-features.h |3 ++-
>  arch/x86/include/asm/required-features.h |3 ++-
>  arch/x86/kernel/cpu/scattered.c  |1 +
>  5 files changed, 14 insertions(+), 5 deletions(-)

...

> diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
> index 8cb57df..d86d9a5 100644
> --- a/arch/x86/kernel/cpu/scattered.c
> +++ b/arch/x86/kernel/cpu/scattered.c
> @@ -37,6 +37,7 @@ void init_scattered_cpuid_features(struct cpuinfo_x86 *c)
>   { X86_FEATURE_HW_PSTATE,CR_EDX, 7, 0x8007, 0 },
>   { X86_FEATURE_CPB,  CR_EDX, 9, 0x8007, 0 },
>   { X86_FEATURE_PROC_FEEDBACK,CR_EDX,11, 0x8007, 0 },
> + { X86_FEATURE_SME,  CR_EAX, 0, 0x801f, 0 },

If this is in scattered CPUID features, it doesn't need any of the
(snipped) changes above - you solely need to reuse one of the free
defines, i.e., something like this:

---
--- a/arch/x86/include/asm/cpufeatures.h2016-09-02 15:49:08.853374323 
+0200
+++ b/arch/x86/include/asm/cpufeatures.h2016-09-02 15:52:34.477365610 
+0200
@@ -100,7 +100,7 @@
 #define X86_FEATURE_XTOPOLOGY  ( 3*32+22) /* cpu topology enum extensions */
 #define X86_FEATURE_TSC_RELIABLE ( 3*32+23) /* TSC is known to be reliable */
 #define X86_FEATURE_NONSTOP_TSC( 3*32+24) /* TSC does not stop in C 
states */
-/* free, was #define X86_FEATURE_CLFLUSH_MONITOR ( 3*32+25) * "" clflush reqd 
with monitor */
+#define X86_FEATURE_SME( 3*32+25) /* Secure Memory Encryption 
*/
 #define X86_FEATURE_EXTD_APICID( 3*32+26) /* has extended APICID (8 
bits) */
 #define X86_FEATURE_AMD_DCM ( 3*32+27) /* multi-node processor */
 #define X86_FEATURE_APERFMPERF ( 3*32+28) /* APERFMPERF */
--- a/arch/x86/kernel/cpu/scattered.c   2016-09-02 15:48:52.753375005 +0200
+++ b/arch/x86/kernel/cpu/scattered.c   2016-09-02 15:51:32.437368239 +0200
@@ -37,6 +37,7 @@ void init_scattered_cpuid_features(struc
{ X86_FEATURE_HW_PSTATE,CR_EDX, 7, 0x8007, 0 },
{ X86_FEATURE_CPB,  CR_EDX, 9, 0x8007, 0 },
{ X86_FEATURE_PROC_FEEDBACK,CR_EDX,11, 0x8007, 0 },
+   { X86_FEATURE_SME,  CR_EAX, 0, 0x801f, 0 },
{ 0, 0, 0, 0, 0 }
};

-- 
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [RFC PATCH v2 05/20] x86: Add the Secure Memory Encryption cpu feature

[Borislav Petkov]

On Mon, Aug 22, 2016 at 05:36:22PM -0500, Tom Lendacky wrote:
> Update the cpu features to include identifying and reporting on the
> Secure Memory Encryption feature.
> 
> Signed-off-by: Tom Lendacky 
> ---
>  arch/x86/include/asm/cpufeature.h|7 +--
>  arch/x86/include/asm/cpufeatures.h   |5 -
>  arch/x86/include/asm/disabled-features.h |3 ++-
>  arch/x86/include/asm/required-features.h |3 ++-
>  arch/x86/kernel/cpu/scattered.c  |1 +
>  5 files changed, 14 insertions(+), 5 deletions(-)

...

> diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
> index 8cb57df..d86d9a5 100644
> --- a/arch/x86/kernel/cpu/scattered.c
> +++ b/arch/x86/kernel/cpu/scattered.c
> @@ -37,6 +37,7 @@ void init_scattered_cpuid_features(struct cpuinfo_x86 *c)
>   { X86_FEATURE_HW_PSTATE,CR_EDX, 7, 0x8007, 0 },
>   { X86_FEATURE_CPB,  CR_EDX, 9, 0x8007, 0 },
>   { X86_FEATURE_PROC_FEEDBACK,CR_EDX,11, 0x8007, 0 },
> + { X86_FEATURE_SME,  CR_EAX, 0, 0x801f, 0 },

If this is in scattered CPUID features, it doesn't need any of the
(snipped) changes above - you solely need to reuse one of the free
defines, i.e., something like this:

---
--- a/arch/x86/include/asm/cpufeatures.h2016-09-02 15:49:08.853374323 
+0200
+++ b/arch/x86/include/asm/cpufeatures.h2016-09-02 15:52:34.477365610 
+0200
@@ -100,7 +100,7 @@
 #define X86_FEATURE_XTOPOLOGY  ( 3*32+22) /* cpu topology enum extensions */
 #define X86_FEATURE_TSC_RELIABLE ( 3*32+23) /* TSC is known to be reliable */
 #define X86_FEATURE_NONSTOP_TSC( 3*32+24) /* TSC does not stop in C 
states */
-/* free, was #define X86_FEATURE_CLFLUSH_MONITOR ( 3*32+25) * "" clflush reqd 
with monitor */
+#define X86_FEATURE_SME( 3*32+25) /* Secure Memory Encryption 
*/
 #define X86_FEATURE_EXTD_APICID( 3*32+26) /* has extended APICID (8 
bits) */
 #define X86_FEATURE_AMD_DCM ( 3*32+27) /* multi-node processor */
 #define X86_FEATURE_APERFMPERF ( 3*32+28) /* APERFMPERF */
--- a/arch/x86/kernel/cpu/scattered.c   2016-09-02 15:48:52.753375005 +0200
+++ b/arch/x86/kernel/cpu/scattered.c   2016-09-02 15:51:32.437368239 +0200
@@ -37,6 +37,7 @@ void init_scattered_cpuid_features(struc
{ X86_FEATURE_HW_PSTATE,CR_EDX, 7, 0x8007, 0 },
{ X86_FEATURE_CPB,  CR_EDX, 9, 0x8007, 0 },
{ X86_FEATURE_PROC_FEEDBACK,CR_EDX,11, 0x8007, 0 },
+   { X86_FEATURE_SME,  CR_EAX, 0, 0x801f, 0 },
{ 0, 0, 0, 0, 0 }
};

-- 
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [Patch v9] driver/clk/clk-si5338: Add common clock framework driver for si5338

[Rob Herring]

On Fri, Aug 26, 2016 at 02:45:49PM -0700, York Sun wrote:
> From: York Sun 
> 
> SI5338 is a programmable clock generator. It has 4 sets of inputs,
> PLL, multisynth and dividers to make 4 outputs. This driver splits
> them into multiple clocks to comply with common clock framework.
> 
> See Documentation/devicetree/bindings/clock/silabs,si5338.txt for
> details.
> 
> Signed-off-by: York Sun 
> CC: Mike Turquette 
> CC: Sebastian Hesselbarth 
> CC: Guenter Roeck 
> CC: Andrey Filippov 
> CC: Paul Bolle 

7 versions before you decide to start cc'ing DT list?

[...]

> diff --git a/Documentation/devicetree/bindings/clock/silabs,si5338.txt 
> b/Documentation/devicetree/bindings/clock/silabs,si5338.txt
> new file mode 100644
> index 000..cc7ae8e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/silabs,si5338.txt
> @@ -0,0 +1,192 @@
> +Binding for Silicon Labs Si5338 programmable i2c clock generator.
> +
> +Reference
> +[1] Si5338 Data Sheet
> +http://www.silabs.com/Support%20Documents/TechnicalDocs/Si5338.pdf
> +
> +The Si5338 is a programmable i2c clock generators with up to 4 output
> +clocks. It has 4 sets of possible input clocks
> +
> +IN1/IN2: differential
> +IN3: single-ended
> +IN4: single-ended
> +IN5/IN6: differential
> +
> +Additionally, IN1/IN2 can be used as XTAL with different setting.
> +The clock tree looks like below (without support of zero-delay)
> +
> +
> +  IN1/IN2 IN3 IN4 IN5/IN6
> + | |   | |
> +   --| |   | |
> +   | | |   | |
> +   | \ /   \ /
> +   |  \   / \   /
> +   |   \ /   \ /
> + XTAL REFCLKFBCLK
> +   |   |  \ /   |
> +   |   |   \   /|
> +   |   | DIVREFCLK DIVFBCLK |
> +   |   | \   /  |
> +   |   |  \ /   |
> +   |   |   \   /|
> +   |   |PLL |
> +   |   |  / | | \   |
> +   |   | /  / \  \  |
> +   |   |/  /   \  \ |
> +   |   |   /   |   |   \|
> +   |   |   |   |   |   ||
> +   |   |  MS0 MS1 MS2 MS3   |
> +   |   |   |   |   |   ||
> +
> +   OUT0  OUT1  OUT2  OUT3
> +
> +The output clock can choose from any of the above clock as its source, with
> +exceptions: MS1 can only be used for OUT1, MS2 can only be used for OUT2, MS3
> +can only be used for OUT3.
> +
> +==I2C device node==
> +
> +Required properties:
> +- compatible: shall be "silabs,si5338".
> +- reg: i2c device address, shall be 0x60, 0x61, 0x70, or 0x71
> +- #clock-cells: shall be set to 1 for multiple outputs
> +- clocks: list of parent clocks in the order of , , , 
> ,
> +  .
> + Note, xtal and in1/2 are mutually exclusive. Only one can be set.
> +- clock-names: The name of the clocks in the same order.
> +- #address-cells: shall be set to 1.
> +- #size-cells: shall be set to 0.
> +- silabs,pll-master: If PLL is used, pick one MS (0, 1, 2, or 3) to allow
> + chaning PLL rate. This is arbitrary since MS0/1/2/3 share one PLL.
> + PLL can be calculated backward to satisfy MS. This node is not
> + required if PLL is not used, or if silabs,pll-vco is set.
> +
> +Optional properties if not set by platform driver:
> +- silab,name-prefix: name prefix for si5338
> +If multiple si5338 chips exist, use name-prefix to form unique names.
> + If omitted, i2c bus name will be used as the prefix.

Drop this please. There should not be any need for it.

> +- silab,ref-source: source of refclk, valid value is defined as
> + #define SI5338_REF_SRC_CLKIN12  0
> + #define SI5338_REF_SRC_CLKIN3   1
> + #define SI5338_REF_SRC_XTAL 4
> +- silab,fb-source:  source of fbclk, valid value is defined as
> + #define SI5338_FB_SRC_CLKIN42
> + #define SI5338_FB_SRC_CLKIN56   3
> + #define SI5338_FB_SRC_NOCLK 5
> +- silabs,pll-source: source of pll, valid value is defined as
> + #define SI5338_PFD_IN_REF_REFCLK   0
> + #define SI5338_PFD_IN_REF_FBCLK1
> + #define SI5338_PFD_IN_REF_DIVREFCLK2
> + #define SI5338_PFD_IN_REF_DIVFBCLK 3
> + #define SI5338_PFD_IN_REF_XOCLK4
> + #define SI5338_PFD_IN_REF_NOCLK5
> +- silabs,pll-vco: Specify VCO frequency for optimal ratios for all outputs.
> + If specified, silabs,pll-master is ignored.
> +
> +==Child nodes==
> +
> +Each of the clock outputs can be configured individually by
> +using a child node to the I2C device node. If a child node for a clock
> +output is not set, platform driver has to set up.
> +
> +Required child node properties:
> +- name: name for the child node
> +It has to be unique. 

Re: [Patch v9] driver/clk/clk-si5338: Add common clock framework driver for si5338

[Rob Herring]

On Fri, Aug 26, 2016 at 02:45:49PM -0700, York Sun wrote:
> From: York Sun 
> 
> SI5338 is a programmable clock generator. It has 4 sets of inputs,
> PLL, multisynth and dividers to make 4 outputs. This driver splits
> them into multiple clocks to comply with common clock framework.
> 
> See Documentation/devicetree/bindings/clock/silabs,si5338.txt for
> details.
> 
> Signed-off-by: York Sun 
> CC: Mike Turquette 
> CC: Sebastian Hesselbarth 
> CC: Guenter Roeck 
> CC: Andrey Filippov 
> CC: Paul Bolle 

7 versions before you decide to start cc'ing DT list?

[...]

> diff --git a/Documentation/devicetree/bindings/clock/silabs,si5338.txt 
> b/Documentation/devicetree/bindings/clock/silabs,si5338.txt
> new file mode 100644
> index 000..cc7ae8e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/silabs,si5338.txt
> @@ -0,0 +1,192 @@
> +Binding for Silicon Labs Si5338 programmable i2c clock generator.
> +
> +Reference
> +[1] Si5338 Data Sheet
> +http://www.silabs.com/Support%20Documents/TechnicalDocs/Si5338.pdf
> +
> +The Si5338 is a programmable i2c clock generators with up to 4 output
> +clocks. It has 4 sets of possible input clocks
> +
> +IN1/IN2: differential
> +IN3: single-ended
> +IN4: single-ended
> +IN5/IN6: differential
> +
> +Additionally, IN1/IN2 can be used as XTAL with different setting.
> +The clock tree looks like below (without support of zero-delay)
> +
> +
> +  IN1/IN2 IN3 IN4 IN5/IN6
> + | |   | |
> +   --| |   | |
> +   | | |   | |
> +   | \ /   \ /
> +   |  \   / \   /
> +   |   \ /   \ /
> + XTAL REFCLKFBCLK
> +   |   |  \ /   |
> +   |   |   \   /|
> +   |   | DIVREFCLK DIVFBCLK |
> +   |   | \   /  |
> +   |   |  \ /   |
> +   |   |   \   /|
> +   |   |PLL |
> +   |   |  / | | \   |
> +   |   | /  / \  \  |
> +   |   |/  /   \  \ |
> +   |   |   /   |   |   \|
> +   |   |   |   |   |   ||
> +   |   |  MS0 MS1 MS2 MS3   |
> +   |   |   |   |   |   ||
> +
> +   OUT0  OUT1  OUT2  OUT3
> +
> +The output clock can choose from any of the above clock as its source, with
> +exceptions: MS1 can only be used for OUT1, MS2 can only be used for OUT2, MS3
> +can only be used for OUT3.
> +
> +==I2C device node==
> +
> +Required properties:
> +- compatible: shall be "silabs,si5338".
> +- reg: i2c device address, shall be 0x60, 0x61, 0x70, or 0x71
> +- #clock-cells: shall be set to 1 for multiple outputs
> +- clocks: list of parent clocks in the order of , , , 
> ,
> +  .
> + Note, xtal and in1/2 are mutually exclusive. Only one can be set.
> +- clock-names: The name of the clocks in the same order.
> +- #address-cells: shall be set to 1.
> +- #size-cells: shall be set to 0.
> +- silabs,pll-master: If PLL is used, pick one MS (0, 1, 2, or 3) to allow
> + chaning PLL rate. This is arbitrary since MS0/1/2/3 share one PLL.
> + PLL can be calculated backward to satisfy MS. This node is not
> + required if PLL is not used, or if silabs,pll-vco is set.
> +
> +Optional properties if not set by platform driver:
> +- silab,name-prefix: name prefix for si5338
> +If multiple si5338 chips exist, use name-prefix to form unique names.
> + If omitted, i2c bus name will be used as the prefix.

Drop this please. There should not be any need for it.

> +- silab,ref-source: source of refclk, valid value is defined as
> + #define SI5338_REF_SRC_CLKIN12  0
> + #define SI5338_REF_SRC_CLKIN3   1
> + #define SI5338_REF_SRC_XTAL 4
> +- silab,fb-source:  source of fbclk, valid value is defined as
> + #define SI5338_FB_SRC_CLKIN42
> + #define SI5338_FB_SRC_CLKIN56   3
> + #define SI5338_FB_SRC_NOCLK 5
> +- silabs,pll-source: source of pll, valid value is defined as
> + #define SI5338_PFD_IN_REF_REFCLK   0
> + #define SI5338_PFD_IN_REF_FBCLK1
> + #define SI5338_PFD_IN_REF_DIVREFCLK2
> + #define SI5338_PFD_IN_REF_DIVFBCLK 3
> + #define SI5338_PFD_IN_REF_XOCLK4
> + #define SI5338_PFD_IN_REF_NOCLK5
> +- silabs,pll-vco: Specify VCO frequency for optimal ratios for all outputs.
> + If specified, silabs,pll-master is ignored.
> +
> +==Child nodes==
> +
> +Each of the clock outputs can be configured individually by
> +using a child node to the I2C device node. If a child node for a clock
> +output is not set, platform driver has to set up.
> +
> +Required child node properties:
> +- name: name for the child node
> +It has to be unique. The name prefix is ignored.
> + If using platform data and the name is not specificed,
> +clkout0/1/2/3 will be used with name prefix.
> +- reg: number 

Re: [PART2 PATCH v7 00/12] iommu/AMD: Introduce IOMMU AVIC support

[Joerg Roedel]

Hi Paolo,

On Fri, Sep 02, 2016 at 12:46:28PM +0200, Paolo Bonzini wrote:
> Joerg, if there's no other issues, could you apply the first 9 patches
> to a branch based on 4.8-rc1 or similar, so that I can pull it into the
> KVM tree?

Sure, I was actually waiting for your Acked-By to put all the
patches into the IOMMU tree, but this will work too :) I'll let you know
when I pushed the branch.


Regards,

Joerg

Re: [PART2 PATCH v7 00/12] iommu/AMD: Introduce IOMMU AVIC support

[Joerg Roedel]

Hi Paolo,

On Fri, Sep 02, 2016 at 12:46:28PM +0200, Paolo Bonzini wrote:
> Joerg, if there's no other issues, could you apply the first 9 patches
> to a branch based on 4.8-rc1 or similar, so that I can pull it into the
> KVM tree?

Sure, I was actually waiting for your Acked-By to put all the
patches into the IOMMU tree, but this will work too :) I'll let you know
when I pushed the branch.


Regards,

Joerg