Re: [PATCH v6 01/14] ACPI: ARM64: IORT: minor cleanup for iort_match_node_callback()

2017-01-03 Thread Hanjun Guo

Hi Lorenzo,

On 2017/1/3 22:08, Lorenzo Pieralisi wrote:

On Mon, Jan 02, 2017 at 09:31:32PM +0800, Hanjun Guo wrote:

Cleanup iort_match_node_callback() a little bit to reduce
some lines of code, aslo fix the indentation in iort_scan_node().


s/aslo/also

"Also" in a commit log is a sign a patch should be split and that's what
you should do even though I know it is tempting to merge all trivial
changes into one single patch.

Make it two patches please.


Will do, thanks!

Do you have more comments regarding this patch set? I will
incorporate all the comments and send out a new version.

Thanks
Hanjun


Re: [PATCH v6 01/14] ACPI: ARM64: IORT: minor cleanup for iort_match_node_callback()

2017-01-03 Thread Hanjun Guo

Hi Lorenzo,

On 2017/1/3 22:08, Lorenzo Pieralisi wrote:

On Mon, Jan 02, 2017 at 09:31:32PM +0800, Hanjun Guo wrote:

Cleanup iort_match_node_callback() a little bit to reduce
some lines of code, aslo fix the indentation in iort_scan_node().


s/aslo/also

"Also" in a commit log is a sign a patch should be split and that's what
you should do even though I know it is tempting to merge all trivial
changes into one single patch.

Make it two patches please.


Will do, thanks!

Do you have more comments regarding this patch set? I will
incorporate all the comments and send out a new version.

Thanks
Hanjun


[v1] misc: genwqe: card_base:-Release memory regions obtained by pci_request_mem_regions

2017-01-03 Thread Arvind Yadav
Free memory regions, if genwqe_bus_reset is not successful.

Signed-off-by: Arvind Yadav 
---
 drivers/misc/genwqe/card_base.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/misc/genwqe/card_base.c b/drivers/misc/genwqe/card_base.c
index 6c1f49a..a998d40 100644
--- a/drivers/misc/genwqe/card_base.c
+++ b/drivers/misc/genwqe/card_base.c
@@ -228,6 +228,7 @@ static int genwqe_bus_reset(struct genwqe_dev *cd)
if (cd->mmio == NULL) {
dev_err(_dev->dev,
"[%s] err: mapping BAR0 failed\n", __func__);
+   pci_release_mem_regions(pci_dev);
return -ENOMEM;
}
return 0;
-- 
1.9.1



[v1] misc: genwqe: card_base:-Release memory regions obtained by pci_request_mem_regions

2017-01-03 Thread Arvind Yadav
Free memory regions, if genwqe_bus_reset is not successful.

Signed-off-by: Arvind Yadav 
---
 drivers/misc/genwqe/card_base.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/misc/genwqe/card_base.c b/drivers/misc/genwqe/card_base.c
index 6c1f49a..a998d40 100644
--- a/drivers/misc/genwqe/card_base.c
+++ b/drivers/misc/genwqe/card_base.c
@@ -228,6 +228,7 @@ static int genwqe_bus_reset(struct genwqe_dev *cd)
if (cd->mmio == NULL) {
dev_err(_dev->dev,
"[%s] err: mapping BAR0 failed\n", __func__);
+   pci_release_mem_regions(pci_dev);
return -ENOMEM;
}
return 0;
-- 
1.9.1



Re: [RFC PATCH] input: Add disable sysfs entry for every input device

2017-01-03 Thread Ivaylo Dimitrov



On  3.01.2017 13:21, Bastien Nocera wrote:

On Mon, 2017-01-02 at 18:09 +0100, Pali Rohár wrote:

On Monday 02 January 2017 16:27:05 Bastien Nocera wrote:

On Sun, 2016-12-25 at 11:04 +0100, Pali Rohár wrote:

This patch allows user to disable events from any input device so
events
would not be delivered to userspace.

Currently there is no way to disable particular input device by
kernel.
User for different reasons would need it for integrated PS/2
keyboard or
touchpad in notebook or touchscreen on mobile device to prevent
sending
events. E.g. mobile phone in pocket or broken integrated PS/2
keyboard.

This is just a RFC patch, not tested yet. Original post about
motivation
about this patch is there: https://lkml.org/lkml/2014/11/29/92


Having implemented something of that ilk in user-space (we
automatically disable touch devices when the associated screen is
turned off/suspended), I think this might need more thought.


How to implement such thing in userspace? I think you cannot do that
without rewriting every one userspace application which uses input.


What happens when a device is opened and the device disabled

through

sysfs, are the users revoked?


Applications will not receive events. Same as if input device does
not
generates events.


Does this put the device in suspend in the same way that closing

the

device's last user does?


Current code not (this is just RFC prototype), but it should be
possible
to implement.


Is this not better implemented in user-space at the session level,
where it knows about which output corresponds to which input

device?

How to do that without rewriting existing applications?


Is this useful enough to disable misbehaving devices on hardware,

so

that the device is not effective on boot?


In case integrated device is absolutely unusable and generates
always
random events, it does not solve problem at boot time.

But more real case is laptop with closed LID press buttons and here
it
is useful.


There's usually a display manager in between the application and the
input device. Whether it's X.org, or a Wayland compositor. Even David's
 https://github.com/dvdhrm/kmscon could help for console applications.



I think the use cases are not clearly explained, will try to:

1. Imagine you have a mobile phone, with a touchscreen, a slide 
keyboard, a keyboard-slide sensor, a proximity sensor and a couple of 
GPIOs, set-up as gpio keys. And you want to carry that phone in your 
pocket, without being worried that it will pick-up an incoming call by 
itself while in the pocket, so:


- slide keyboard is closed, you "lock" the phone before put it in your 
pocket - in that state, touchscreen and most of the gpio-keys should be 
"disabled", so no touches are registered waking-up the device without need.
- a call comes, proximity gets "enabled", but TS should stay disabled as 
proximity detects "the phone is in a pocket"
- you get your phone out of your pocket - proximity detects no more 
obstacles, so now TS has to be enabled giving you a chance to pick up 
the incoming call.


"disabling" of gpio-keys is clear, but how to make TS and proximity 
inactive when needed? Sure, touches can be simply ignored (by using 
xinput "Device Enabled" 0 on x11), same for proximity, but keep in mind 
this is a battery-operated device, so we don't want CPU wake-ups with no 
need.


2. The same device, "locked", but this time with slide keyboard opened:

- both keyboard and TS should be "disabled" so no touches neither key 
presses wake-up the system. Only the power-button (or some other, 
doesn't matter) should be enabled to activate the device.


There are more use-cases similar to the above as well as use-cases for 
laptops, but I hope you're getting the idea.


Also, the interface to "disable" an input devices should be independent 
to whether you use X11, wayland or your application draws directly to 
the framebuffer.


Ivo


Re: [RFC PATCH] input: Add disable sysfs entry for every input device

2017-01-03 Thread Ivaylo Dimitrov



On  3.01.2017 13:21, Bastien Nocera wrote:

On Mon, 2017-01-02 at 18:09 +0100, Pali Rohár wrote:

On Monday 02 January 2017 16:27:05 Bastien Nocera wrote:

On Sun, 2016-12-25 at 11:04 +0100, Pali Rohár wrote:

This patch allows user to disable events from any input device so
events
would not be delivered to userspace.

Currently there is no way to disable particular input device by
kernel.
User for different reasons would need it for integrated PS/2
keyboard or
touchpad in notebook or touchscreen on mobile device to prevent
sending
events. E.g. mobile phone in pocket or broken integrated PS/2
keyboard.

This is just a RFC patch, not tested yet. Original post about
motivation
about this patch is there: https://lkml.org/lkml/2014/11/29/92


Having implemented something of that ilk in user-space (we
automatically disable touch devices when the associated screen is
turned off/suspended), I think this might need more thought.


How to implement such thing in userspace? I think you cannot do that
without rewriting every one userspace application which uses input.


What happens when a device is opened and the device disabled

through

sysfs, are the users revoked?


Applications will not receive events. Same as if input device does
not
generates events.


Does this put the device in suspend in the same way that closing

the

device's last user does?


Current code not (this is just RFC prototype), but it should be
possible
to implement.


Is this not better implemented in user-space at the session level,
where it knows about which output corresponds to which input

device?

How to do that without rewriting existing applications?


Is this useful enough to disable misbehaving devices on hardware,

so

that the device is not effective on boot?


In case integrated device is absolutely unusable and generates
always
random events, it does not solve problem at boot time.

But more real case is laptop with closed LID press buttons and here
it
is useful.


There's usually a display manager in between the application and the
input device. Whether it's X.org, or a Wayland compositor. Even David's
 https://github.com/dvdhrm/kmscon could help for console applications.



I think the use cases are not clearly explained, will try to:

1. Imagine you have a mobile phone, with a touchscreen, a slide 
keyboard, a keyboard-slide sensor, a proximity sensor and a couple of 
GPIOs, set-up as gpio keys. And you want to carry that phone in your 
pocket, without being worried that it will pick-up an incoming call by 
itself while in the pocket, so:


- slide keyboard is closed, you "lock" the phone before put it in your 
pocket - in that state, touchscreen and most of the gpio-keys should be 
"disabled", so no touches are registered waking-up the device without need.
- a call comes, proximity gets "enabled", but TS should stay disabled as 
proximity detects "the phone is in a pocket"
- you get your phone out of your pocket - proximity detects no more 
obstacles, so now TS has to be enabled giving you a chance to pick up 
the incoming call.


"disabling" of gpio-keys is clear, but how to make TS and proximity 
inactive when needed? Sure, touches can be simply ignored (by using 
xinput "Device Enabled" 0 on x11), same for proximity, but keep in mind 
this is a battery-operated device, so we don't want CPU wake-ups with no 
need.


2. The same device, "locked", but this time with slide keyboard opened:

- both keyboard and TS should be "disabled" so no touches neither key 
presses wake-up the system. Only the power-button (or some other, 
doesn't matter) should be enabled to activate the device.


There are more use-cases similar to the above as well as use-cases for 
laptops, but I hope you're getting the idea.


Also, the interface to "disable" an input devices should be independent 
to whether you use X11, wayland or your application draws directly to 
the framebuffer.


Ivo


Re: [PATCH 2/7] mm, vmscan: add active list aging tracepoint

2017-01-03 Thread Michal Hocko
On Wed 04-01-17 14:07:22, Minchan Kim wrote:
> On Tue, Jan 03, 2017 at 09:21:22AM +0100, Michal Hocko wrote:
[...]
> > with other tracepoints but that can be helpful because you do not have
> > all the tracepoints enabled all the time. So unless you see this
> > particular thing as a road block I would rather keep it.
> 
> I didn't know how long this thread becomes lenghy. To me, it was no worth
> to discuss. I did best effot to explain my stand with valid points, I think
> and don't want to go infinite loop. If you don't agree still, separate
> the patch. One includes only necessary things with removing nr_scanned, which
> I am happy to ack it. Based upon it, add one more patch you want adding
> nr_scanned with your claim. I will reply that thread with my claim and
> let's keep an eye on it that whether maintainer will take it or not.

To be honest this is just not worth the effort and rather than
discussing further I will just drop the nr_scanned slthough I disagree
that your concerns regarding this _particular counter_ are really valid.

> If maintainer will take it, it's good indication which will represent
> we can add more extra tracepoint easily with "might be helpful with someone
> although it's redunant" so do not prevent others who want to do
> in the future.

no we do not work in a precedence system like that.

-- 
Michal Hocko
SUSE Labs


Re: [PATCH 2/7] mm, vmscan: add active list aging tracepoint

2017-01-03 Thread Michal Hocko
On Wed 04-01-17 14:07:22, Minchan Kim wrote:
> On Tue, Jan 03, 2017 at 09:21:22AM +0100, Michal Hocko wrote:
[...]
> > with other tracepoints but that can be helpful because you do not have
> > all the tracepoints enabled all the time. So unless you see this
> > particular thing as a road block I would rather keep it.
> 
> I didn't know how long this thread becomes lenghy. To me, it was no worth
> to discuss. I did best effot to explain my stand with valid points, I think
> and don't want to go infinite loop. If you don't agree still, separate
> the patch. One includes only necessary things with removing nr_scanned, which
> I am happy to ack it. Based upon it, add one more patch you want adding
> nr_scanned with your claim. I will reply that thread with my claim and
> let's keep an eye on it that whether maintainer will take it or not.

To be honest this is just not worth the effort and rather than
discussing further I will just drop the nr_scanned slthough I disagree
that your concerns regarding this _particular counter_ are really valid.

> If maintainer will take it, it's good indication which will represent
> we can add more extra tracepoint easily with "might be helpful with someone
> although it's redunant" so do not prevent others who want to do
> in the future.

no we do not work in a precedence system like that.

-- 
Michal Hocko
SUSE Labs


Re: [PATCH] virtio_blk: avoid DMA to stack for the sense buffer

2017-01-03 Thread Jason Wang



On 2017年01月04日 13:25, Christoph Hellwig wrote:

Most users of BLOCK_PC requests allocate the sense buffer on the stack,
so to avoid DMA to the stack copy them to a field in the heap allocated
virtblk_req structure.  Without that any attempt at SCSI passthrough I/O,
including the SG_IO ioctl from userspace will crash the kernel.  Note that
this includes running tools like hdparm even when the host does not have
SCSI passthrough enabled.

Signed-off-by: Christoph Hellwig 
---
  drivers/block/virtio_blk.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 5545a67..3c3b8f6 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -56,6 +56,7 @@ struct virtblk_req {
struct virtio_blk_outhdr out_hdr;
struct virtio_scsi_inhdr in_hdr;
u8 status;
+   u8 sense[SCSI_SENSE_BUFFERSIZE];
struct scatterlist sg[];
  };
  
@@ -102,7 +103,8 @@ static int __virtblk_add_req(struct virtqueue *vq,

}
  
  	if (type == cpu_to_virtio32(vq->vdev, VIRTIO_BLK_T_SCSI_CMD)) {

-   sg_init_one(, vbr->req->sense, SCSI_SENSE_BUFFERSIZE);
+   memcpy(vbr->sense, vbr->req->sense, SCSI_SENSE_BUFFERSIZE);
+   sg_init_one(, vbr->sense, SCSI_SENSE_BUFFERSIZE);
sgs[num_out + num_in++] = 
sg_init_one(, >in_hdr, sizeof(vbr->in_hdr));
sgs[num_out + num_in++] = 


Acked-by: Jason Wang 


Re: [PATCH] virtio_blk: avoid DMA to stack for the sense buffer

2017-01-03 Thread Jason Wang



On 2017年01月04日 13:25, Christoph Hellwig wrote:

Most users of BLOCK_PC requests allocate the sense buffer on the stack,
so to avoid DMA to the stack copy them to a field in the heap allocated
virtblk_req structure.  Without that any attempt at SCSI passthrough I/O,
including the SG_IO ioctl from userspace will crash the kernel.  Note that
this includes running tools like hdparm even when the host does not have
SCSI passthrough enabled.

Signed-off-by: Christoph Hellwig 
---
  drivers/block/virtio_blk.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 5545a67..3c3b8f6 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -56,6 +56,7 @@ struct virtblk_req {
struct virtio_blk_outhdr out_hdr;
struct virtio_scsi_inhdr in_hdr;
u8 status;
+   u8 sense[SCSI_SENSE_BUFFERSIZE];
struct scatterlist sg[];
  };
  
@@ -102,7 +103,8 @@ static int __virtblk_add_req(struct virtqueue *vq,

}
  
  	if (type == cpu_to_virtio32(vq->vdev, VIRTIO_BLK_T_SCSI_CMD)) {

-   sg_init_one(, vbr->req->sense, SCSI_SENSE_BUFFERSIZE);
+   memcpy(vbr->sense, vbr->req->sense, SCSI_SENSE_BUFFERSIZE);
+   sg_init_one(, vbr->sense, SCSI_SENSE_BUFFERSIZE);
sgs[num_out + num_in++] = 
sg_init_one(, >in_hdr, sizeof(vbr->in_hdr));
sgs[num_out + num_in++] = 


Acked-by: Jason Wang 


RE: [PATCH v2 2/3] dmaeninge: xilinx_dma: Fix bug in multiple frame stores scenario in vdma

2017-01-03 Thread Appana Durga Kedareswara Rao
Hi Jose Miguel Abreu,

Thanks for the review...

> >> If so then there is no race condition, but the HW image that I have
> >> does not have this register enabled so I was getting this result
> >> (memory corruption because not all framebuffers had addresses set).
> > Thanks for the explanation.
> > Agree the issue that you mentioned won't come when
> > XILINX_DMA_REG_FRMSTORE
> > (C_ENABLE_DEBUG_INFO_5 and C_ENABLE_DEBUG_INFO_13) Register is
> enabled in the IP.
> > But this register won't get enabled with the default IP configuration
> (C_ENABLE_DEBUG_INFO_5 and C_ENABLE_DEBUG_INFO_13).
> >
> > When user is not enabled XILINX_DMA_REG_FRMSTORE in the h/w and
> submits frames less than h/w capable.
> > The solution that I am thinking is to throw an error in the driver
> > saying that either enable the num frame store feature in the IP or submit 
> > the
> frames up to h/w capable what do you think???
> 
> Sounds fine by me.

Thanks posted the v3 series please review when you have some time...

Regards,
Kedar.

> 
> Best regards,
> Jose Miguel Abreu
> 
> >
> > Regards,
> > Kedar.
> >
> >> Best regards,
> >> Jose Miguel Abreu
> >>
> >>> Regards,
> >>> Kedar.
> >>>
>  Best regards,
>  Jose Miguel Abreu
> 



RE: [PATCH v2 2/3] dmaeninge: xilinx_dma: Fix bug in multiple frame stores scenario in vdma

2017-01-03 Thread Appana Durga Kedareswara Rao
Hi Jose Miguel Abreu,

Thanks for the review...

> >> If so then there is no race condition, but the HW image that I have
> >> does not have this register enabled so I was getting this result
> >> (memory corruption because not all framebuffers had addresses set).
> > Thanks for the explanation.
> > Agree the issue that you mentioned won't come when
> > XILINX_DMA_REG_FRMSTORE
> > (C_ENABLE_DEBUG_INFO_5 and C_ENABLE_DEBUG_INFO_13) Register is
> enabled in the IP.
> > But this register won't get enabled with the default IP configuration
> (C_ENABLE_DEBUG_INFO_5 and C_ENABLE_DEBUG_INFO_13).
> >
> > When user is not enabled XILINX_DMA_REG_FRMSTORE in the h/w and
> submits frames less than h/w capable.
> > The solution that I am thinking is to throw an error in the driver
> > saying that either enable the num frame store feature in the IP or submit 
> > the
> frames up to h/w capable what do you think???
> 
> Sounds fine by me.

Thanks posted the v3 series please review when you have some time...

Regards,
Kedar.

> 
> Best regards,
> Jose Miguel Abreu
> 
> >
> > Regards,
> > Kedar.
> >
> >> Best regards,
> >> Jose Miguel Abreu
> >>
> >>> Regards,
> >>> Kedar.
> >>>
>  Best regards,
>  Jose Miguel Abreu
> 



Re: [PATCH v4 1/3] drm/exynos: mic: Add mode_set callback function

2017-01-03 Thread Inki Dae


2017년 01월 04일 15:58에 Hoegeun Kwon 이(가) 쓴 글:
> Before applying the patch, used the of_get_videomode function to
> parse the display-timings in the panel which is the child driver
> of dsi in the devicetree. this is wrong. So removed the
> of_get_videomode and fixed to get videomode struct through
> mode_set callback function.
> 
> Signed-off-by: Hoegeun Kwon 
> ---
>  drivers/gpu/drm/exynos/exynos_drm_mic.c | 33 
> ++---
>  1 file changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c 
> b/drivers/gpu/drm/exynos/exynos_drm_mic.c
> index a0def0b..9a50ceb 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_mic.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c
> @@ -286,13 +286,6 @@ static int parse_dt(struct exynos_mic *mic)
>   }
>   nodes[j++] = remote_node;
>  
> - ret = of_get_videomode(remote_node,
> - >vm, 0);
> - if (ret) {
> - DRM_ERROR("mic: failed to get videomode");
> - goto exit;
> - }
> -
>   break;
>   default:
>   DRM_ERROR("mic: Unknown endpoint from MIC");
> @@ -329,6 +322,27 @@ static void mic_post_disable(struct drm_bridge *bridge)
>   mutex_unlock(_mutex);
>  }
>  
> +static void mic_mode_set(struct drm_bridge *bridge,
> + struct drm_display_mode *mode,
> + struct drm_display_mode *adjusted_mode)
> +{
> + struct exynos_mic *mic = bridge->driver_private;
> +
> + mutex_lock(_mutex);
> + if (mic->enabled)
> + goto already_enabled;

mode setting should be performed every time mode_set callback is called so 
remove above two lines.

> +
> + drm_display_mode_to_videomode(mode, >vm);
> +
> + if (!mic->i80_mode)
> + mic_set_porch_timing(mic);
> + mic_set_img_size(mic);
> + mic_set_output_timing(mic);
> +
> +already_enabled:

So this label is unnecessary.

> + mutex_unlock(_mutex);
> +}
> +
>  static void mic_pre_enable(struct drm_bridge *bridge)
>  {
>   struct exynos_mic *mic = bridge->driver_private;
> @@ -355,10 +369,6 @@ static void mic_pre_enable(struct drm_bridge *bridge)
>   goto turn_off_clks;
>   }
>  
> - if (!mic->i80_mode)
> - mic_set_porch_timing(mic);
> - mic_set_img_size(mic);
> - mic_set_output_timing(mic);
>   mic_set_reg_on(mic, 1);
>   mic->enabled = 1;
>   mutex_unlock(_mutex);
> @@ -377,6 +387,7 @@ static void mic_enable(struct drm_bridge *bridge) { }
>  static const struct drm_bridge_funcs mic_bridge_funcs = {
>   .disable = mic_disable,
>   .post_disable = mic_post_disable,
> + .mode_set = mic_mode_set,
>   .pre_enable = mic_pre_enable,
>   .enable = mic_enable,
>  };
> 


Re: [PATCH v4 1/3] drm/exynos: mic: Add mode_set callback function

2017-01-03 Thread Inki Dae


2017년 01월 04일 15:58에 Hoegeun Kwon 이(가) 쓴 글:
> Before applying the patch, used the of_get_videomode function to
> parse the display-timings in the panel which is the child driver
> of dsi in the devicetree. this is wrong. So removed the
> of_get_videomode and fixed to get videomode struct through
> mode_set callback function.
> 
> Signed-off-by: Hoegeun Kwon 
> ---
>  drivers/gpu/drm/exynos/exynos_drm_mic.c | 33 
> ++---
>  1 file changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c 
> b/drivers/gpu/drm/exynos/exynos_drm_mic.c
> index a0def0b..9a50ceb 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_mic.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c
> @@ -286,13 +286,6 @@ static int parse_dt(struct exynos_mic *mic)
>   }
>   nodes[j++] = remote_node;
>  
> - ret = of_get_videomode(remote_node,
> - >vm, 0);
> - if (ret) {
> - DRM_ERROR("mic: failed to get videomode");
> - goto exit;
> - }
> -
>   break;
>   default:
>   DRM_ERROR("mic: Unknown endpoint from MIC");
> @@ -329,6 +322,27 @@ static void mic_post_disable(struct drm_bridge *bridge)
>   mutex_unlock(_mutex);
>  }
>  
> +static void mic_mode_set(struct drm_bridge *bridge,
> + struct drm_display_mode *mode,
> + struct drm_display_mode *adjusted_mode)
> +{
> + struct exynos_mic *mic = bridge->driver_private;
> +
> + mutex_lock(_mutex);
> + if (mic->enabled)
> + goto already_enabled;

mode setting should be performed every time mode_set callback is called so 
remove above two lines.

> +
> + drm_display_mode_to_videomode(mode, >vm);
> +
> + if (!mic->i80_mode)
> + mic_set_porch_timing(mic);
> + mic_set_img_size(mic);
> + mic_set_output_timing(mic);
> +
> +already_enabled:

So this label is unnecessary.

> + mutex_unlock(_mutex);
> +}
> +
>  static void mic_pre_enable(struct drm_bridge *bridge)
>  {
>   struct exynos_mic *mic = bridge->driver_private;
> @@ -355,10 +369,6 @@ static void mic_pre_enable(struct drm_bridge *bridge)
>   goto turn_off_clks;
>   }
>  
> - if (!mic->i80_mode)
> - mic_set_porch_timing(mic);
> - mic_set_img_size(mic);
> - mic_set_output_timing(mic);
>   mic_set_reg_on(mic, 1);
>   mic->enabled = 1;
>   mutex_unlock(_mutex);
> @@ -377,6 +387,7 @@ static void mic_enable(struct drm_bridge *bridge) { }
>  static const struct drm_bridge_funcs mic_bridge_funcs = {
>   .disable = mic_disable,
>   .post_disable = mic_post_disable,
> + .mode_set = mic_mode_set,
>   .pre_enable = mic_pre_enable,
>   .enable = mic_enable,
>  };
> 


Re: [PATCH v4 2/3] drm/panel: Add support for S6E3HA2 panel driver on TM2 board

2017-01-03 Thread Chanwoo Choi
Hi Hoegeun,

I tested this patch on Exynos5433-TM2 board. It is well working

Tested-by: Chanwoo Choi 

Regards,
Chanwoo Choi

On 2017년 01월 04일 15:58, Hoegeun Kwon wrote:
> This patch add support for MIPI-DSI based S6E3HA2 AMOLED panel
> driver. This panel has 1440x2560 resolution in 5.7-inch physical
> panel in the TM2 device.
> 
> Signed-off-by: Donghwa Lee 
> Signed-off-by: Hyungwon Hwang 
> Signed-off-by: Hoegeun Kwon 
> ---
>  .../bindings/display/panel/samsung,s6e3ha2.txt |  40 ++
>  drivers/gpu/drm/panel/Kconfig  |   6 +
>  drivers/gpu/drm/panel/Makefile |   1 +
>  drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c  | 741 
> +
>  4 files changed, 788 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt
>  create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt 
> b/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt
> new file mode 100644
> index 000..6879f51
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt
> @@ -0,0 +1,40 @@
> +Samsung S6E3HA2 5.7" 1440x2560 AMOLED panel
> +
> +Required properties:
> +  - compatible: "samsung,s6e3ha2"
> +  - reg: the virtual channel number of a DSI peripheral
> +  - vdd3-supply: I/O voltage supply
> +  - vci-supply: voltage supply for analog circuits
> +  - reset-gpios: a GPIO spec for the reset pin (active low)
> +  - enable-gpios: a GPIO spec for the panel enable pin (active high)
> +  - te-gpios: a GPIO spec for the tearing effect synchronization signal
> +gpio pin (active high)
> +
> +The device node can contain one 'port' child node with one child
> +'endpoint' node, according to the bindings defined in [1]. This
> +node should describe panel's video bus.
> +
> +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
> +
> +Example:
> +
> + {
> + ...
> +
> + panel@0 {
> + compatible = "samsung,s6e3ha2";
> + reg = <0>;
> + vdd3-supply = <_reg>;
> + vci-supply = <_reg>;
> + reset-gpios = < 0 GPIO_ACTIVE_LOW>;
> + enable-gpios = < 5 GPIO_ACTIVE_HIGH>;
> + te-gpios = < 3 GPIO_ACTIVE_HIGH>;
> +
> + port {
> + panel_in: endpoint {
> + remote-endpoint = <_out>;
> + };
> + };
> + };
> +};
> +
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 62aba97..eea2902 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -52,6 +52,12 @@ config DRM_PANEL_PANASONIC_VVX10F034N00
> WUXGA (1920x1200) Novatek NT1397-based DSI panel as found in some
> Xperia Z2 tablets
>  
> +config DRM_PANEL_SAMSUNG_S6E3HA2
> + tristate "Samsung S6E3HA2 DSI video mode panel"
> + depends on OF
> + depends on  DRM_MIPI_DSI
> + select VIDEOMODE_HELPERS
> +
>  config DRM_PANEL_SAMSUNG_S6E8AA0
>   tristate "Samsung S6E8AA0 DSI video mode panel"
>   depends on OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index a5c7ec0..1d483b0 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -3,6 +3,7 @@ obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += 
> panel-jdi-lt070me05000.o
>  obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
>  obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) += 
> panel-panasonic-vvx10f034n00.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
> +obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA2) += panel-samsung-s6e3ha2.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0) += panel-samsung-s6e8aa0.o
>  obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
>  obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
> diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c 
> b/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
> new file mode 100644
> index 000..8c5a1c2
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
> @@ -0,0 +1,741 @@
> +/*
> + * MIPI-DSI based s6e3ha2 AMOLED 5.7 inch panel driver.
> + *
> + * Copyright (c) 2016 Samsung Electronics Co., Ltd.
> + * Donghwa Lee 
> + * Hyungwon Hwang 
> + * Hoegeun Kwon 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define S6E3HA2_MIN_BRIGHTNESS   0
> +#define S6E3HA2_MAX_BRIGHTNESS   100
> +#define 

Re: [PATCH v4 2/3] drm/panel: Add support for S6E3HA2 panel driver on TM2 board

2017-01-03 Thread Chanwoo Choi
Hi Hoegeun,

I tested this patch on Exynos5433-TM2 board. It is well working

Tested-by: Chanwoo Choi 

Regards,
Chanwoo Choi

On 2017년 01월 04일 15:58, Hoegeun Kwon wrote:
> This patch add support for MIPI-DSI based S6E3HA2 AMOLED panel
> driver. This panel has 1440x2560 resolution in 5.7-inch physical
> panel in the TM2 device.
> 
> Signed-off-by: Donghwa Lee 
> Signed-off-by: Hyungwon Hwang 
> Signed-off-by: Hoegeun Kwon 
> ---
>  .../bindings/display/panel/samsung,s6e3ha2.txt |  40 ++
>  drivers/gpu/drm/panel/Kconfig  |   6 +
>  drivers/gpu/drm/panel/Makefile |   1 +
>  drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c  | 741 
> +
>  4 files changed, 788 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt
>  create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt 
> b/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt
> new file mode 100644
> index 000..6879f51
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt
> @@ -0,0 +1,40 @@
> +Samsung S6E3HA2 5.7" 1440x2560 AMOLED panel
> +
> +Required properties:
> +  - compatible: "samsung,s6e3ha2"
> +  - reg: the virtual channel number of a DSI peripheral
> +  - vdd3-supply: I/O voltage supply
> +  - vci-supply: voltage supply for analog circuits
> +  - reset-gpios: a GPIO spec for the reset pin (active low)
> +  - enable-gpios: a GPIO spec for the panel enable pin (active high)
> +  - te-gpios: a GPIO spec for the tearing effect synchronization signal
> +gpio pin (active high)
> +
> +The device node can contain one 'port' child node with one child
> +'endpoint' node, according to the bindings defined in [1]. This
> +node should describe panel's video bus.
> +
> +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
> +
> +Example:
> +
> + {
> + ...
> +
> + panel@0 {
> + compatible = "samsung,s6e3ha2";
> + reg = <0>;
> + vdd3-supply = <_reg>;
> + vci-supply = <_reg>;
> + reset-gpios = < 0 GPIO_ACTIVE_LOW>;
> + enable-gpios = < 5 GPIO_ACTIVE_HIGH>;
> + te-gpios = < 3 GPIO_ACTIVE_HIGH>;
> +
> + port {
> + panel_in: endpoint {
> + remote-endpoint = <_out>;
> + };
> + };
> + };
> +};
> +
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 62aba97..eea2902 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -52,6 +52,12 @@ config DRM_PANEL_PANASONIC_VVX10F034N00
> WUXGA (1920x1200) Novatek NT1397-based DSI panel as found in some
> Xperia Z2 tablets
>  
> +config DRM_PANEL_SAMSUNG_S6E3HA2
> + tristate "Samsung S6E3HA2 DSI video mode panel"
> + depends on OF
> + depends on  DRM_MIPI_DSI
> + select VIDEOMODE_HELPERS
> +
>  config DRM_PANEL_SAMSUNG_S6E8AA0
>   tristate "Samsung S6E8AA0 DSI video mode panel"
>   depends on OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index a5c7ec0..1d483b0 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -3,6 +3,7 @@ obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += 
> panel-jdi-lt070me05000.o
>  obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
>  obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) += 
> panel-panasonic-vvx10f034n00.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
> +obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA2) += panel-samsung-s6e3ha2.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0) += panel-samsung-s6e8aa0.o
>  obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
>  obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
> diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c 
> b/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
> new file mode 100644
> index 000..8c5a1c2
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
> @@ -0,0 +1,741 @@
> +/*
> + * MIPI-DSI based s6e3ha2 AMOLED 5.7 inch panel driver.
> + *
> + * Copyright (c) 2016 Samsung Electronics Co., Ltd.
> + * Donghwa Lee 
> + * Hyungwon Hwang 
> + * Hoegeun Kwon 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define S6E3HA2_MIN_BRIGHTNESS   0
> +#define S6E3HA2_MAX_BRIGHTNESS   100
> +#define S6E3HA2_DEFAULT_BRIGHTNESS   80
> +
> +#define S6E3HA2_NUM_GAMMA_STEPS  46
> +#define S6E3HA2_GAMMA_CMD_CNT35
> +#define S6E3HA2_VINT_STATUS_MAX   

Re: [PATCH 2/7] mm, vmscan: add active list aging tracepoint

2017-01-03 Thread Vlastimil Babka
On 01/04/2017 06:07 AM, Minchan Kim wrote:
> With this,
> ./scripts/bloat-o-meter vmlinux.old vmlinux.new.new
> add/remove: 1/1 grow/shrink: 0/9 up/down: 1394/-1636 (-242)
> function old new   delta
> isolate_lru_pages  -1394   +1394
> print_fmt_mm_vmscan_lru_shrink_inactive  359 355  -4
> vermagic  64  58  -6
> perf_trace_mm_vmscan_lru_shrink_active   264 256  -8
> trace_raw_output_mm_vmscan_lru_shrink_active 203 193 -10
> trace_event_raw_event_mm_vmscan_lru_shrink_active 241 225 -16
> print_fmt_mm_vmscan_lru_shrink_active458 426 -32
> trace_event_define_fields_mm_vmscan_lru_shrink_active 384 336 -48
> shrink_inactive_list14301271-159
> shrink_active_list  12651082-183
> isolate_lru_pages.isra  1170   -   -1170
> Total: Before=26268743, After=26268501, chg -0.00%
> 
> We can save 242 bytes.
> 
> If we consider binary size, 424 bytes save.
> 
> #> ls -l vmlinux.old vmlinux.new.new
> 194092840  vmlinux.old
> 194092416  vmlinux.new.new

Which is roughly 0.0002%. Not that I'm against fighting bloat, but let's
not forget that it's not the only factor. For example the following part
from above:

> isolate_lru_pages  -1394   +1394
> isolate_lru_pages.isra  1170   -   -1170

shows that your change has prevented a -fipa-src gcc optimisation, which
is "interprocedural scalar replacement of aggregates, removal of unused
parameters and replacement of parameters passed by reference by
parameters passed by value." Well, I'm no gcc expert :) but it might be
that the change is not a simple win-win.


Re: [PATCH v6 06/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_init() to prepare for ACPI

2017-01-03 Thread Tomasz Nowicki

On 04.01.2017 08:02, Hanjun Guo wrote:

Hi Tomasz,

On 2017/1/3 15:41, Tomasz Nowicki wrote:

Hi,

Can we merge patch 4 & 6 into one patch so that we keep refactoring part
as one piece ? I do not see a reason to keep them separate or have patch
5 in between. You can refactor what needs to be refactored, add
necessary functions to iort.c and then support ACPI for
irq-gic-v3-its-platform-msi.c


There are two functions here,
 - retrieve the dev id from IORT which was DT based only;

 - init the platform msi domain from MADT;

For each of them split it into two steps,
 - refactor the code for ACPI later and it's easy for review
   because wen can easily to figure out it has functional
   change or not

 - add ACPI functionality

Does it make sense?


It is up to Marc, but personally I prefer:
1. Refactor dev id retrieving and init function in one patch and 
highlight no functional changes in changelog

2. Crate necessary infrastructure in iort.c
3. Then add ACPI support to irq-gic-v3-its-platform-msi.c

Thanks,
Tomasz


Re: [PATCH 2/7] mm, vmscan: add active list aging tracepoint

2017-01-03 Thread Vlastimil Babka
On 01/04/2017 06:07 AM, Minchan Kim wrote:
> With this,
> ./scripts/bloat-o-meter vmlinux.old vmlinux.new.new
> add/remove: 1/1 grow/shrink: 0/9 up/down: 1394/-1636 (-242)
> function old new   delta
> isolate_lru_pages  -1394   +1394
> print_fmt_mm_vmscan_lru_shrink_inactive  359 355  -4
> vermagic  64  58  -6
> perf_trace_mm_vmscan_lru_shrink_active   264 256  -8
> trace_raw_output_mm_vmscan_lru_shrink_active 203 193 -10
> trace_event_raw_event_mm_vmscan_lru_shrink_active 241 225 -16
> print_fmt_mm_vmscan_lru_shrink_active458 426 -32
> trace_event_define_fields_mm_vmscan_lru_shrink_active 384 336 -48
> shrink_inactive_list14301271-159
> shrink_active_list  12651082-183
> isolate_lru_pages.isra  1170   -   -1170
> Total: Before=26268743, After=26268501, chg -0.00%
> 
> We can save 242 bytes.
> 
> If we consider binary size, 424 bytes save.
> 
> #> ls -l vmlinux.old vmlinux.new.new
> 194092840  vmlinux.old
> 194092416  vmlinux.new.new

Which is roughly 0.0002%. Not that I'm against fighting bloat, but let's
not forget that it's not the only factor. For example the following part
from above:

> isolate_lru_pages  -1394   +1394
> isolate_lru_pages.isra  1170   -   -1170

shows that your change has prevented a -fipa-src gcc optimisation, which
is "interprocedural scalar replacement of aggregates, removal of unused
parameters and replacement of parameters passed by reference by
parameters passed by value." Well, I'm no gcc expert :) but it might be
that the change is not a simple win-win.


Re: [PATCH v6 06/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_init() to prepare for ACPI

2017-01-03 Thread Tomasz Nowicki

On 04.01.2017 08:02, Hanjun Guo wrote:

Hi Tomasz,

On 2017/1/3 15:41, Tomasz Nowicki wrote:

Hi,

Can we merge patch 4 & 6 into one patch so that we keep refactoring part
as one piece ? I do not see a reason to keep them separate or have patch
5 in between. You can refactor what needs to be refactored, add
necessary functions to iort.c and then support ACPI for
irq-gic-v3-its-platform-msi.c


There are two functions here,
 - retrieve the dev id from IORT which was DT based only;

 - init the platform msi domain from MADT;

For each of them split it into two steps,
 - refactor the code for ACPI later and it's easy for review
   because wen can easily to figure out it has functional
   change or not

 - add ACPI functionality

Does it make sense?


It is up to Marc, but personally I prefer:
1. Refactor dev id retrieving and init function in one patch and 
highlight no functional changes in changelog

2. Crate necessary infrastructure in iort.c
3. Then add ACPI support to irq-gic-v3-its-platform-msi.c

Thanks,
Tomasz


[PATCH v3 1/3] dmaengine: xilinx_dma: Check for channel idle state before submitting dma descriptor

2017-01-03 Thread Kedareswara rao Appana
Add channel idle state to ensure that dma descriptor is not
submitted when VDMA engine is in progress.

Reviewed-by: Jose Abreu 
Signed-off-by: Kedareswara rao Appana 
---
Changes for v3:
---> None.
Changes for v2:
---> Add idle check in the reset as suggested by Jose Abreu
---> Removed xilinx_dma_is_running/xilinx_dma_is_idle checks
in the driver and used common idle checks across the driver
as suggested by Laurent Pinchart.

 drivers/dma/xilinx/xilinx_dma.c | 56 +
 1 file changed, 17 insertions(+), 39 deletions(-)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 8288fe4..be7eb41 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -321,6 +321,7 @@ struct xilinx_dma_tx_descriptor {
  * @cyclic: Check for cyclic transfers.
  * @genlock: Support genlock mode
  * @err: Channel has errors
+ * @idle: Check for channel idle
  * @tasklet: Cleanup work after irq
  * @config: Device configuration info
  * @flush_on_fsync: Flush on Frame sync
@@ -351,6 +352,7 @@ struct xilinx_dma_chan {
bool cyclic;
bool genlock;
bool err;
+   bool idle;
struct tasklet_struct tasklet;
struct xilinx_vdma_config config;
bool flush_on_fsync;
@@ -920,32 +922,6 @@ static enum dma_status xilinx_dma_tx_status(struct 
dma_chan *dchan,
 }
 
 /**
- * xilinx_dma_is_running - Check if DMA channel is running
- * @chan: Driver specific DMA channel
- *
- * Return: '1' if running, '0' if not.
- */
-static bool xilinx_dma_is_running(struct xilinx_dma_chan *chan)
-{
-   return !(dma_ctrl_read(chan, XILINX_DMA_REG_DMASR) &
-XILINX_DMA_DMASR_HALTED) &&
-   (dma_ctrl_read(chan, XILINX_DMA_REG_DMACR) &
-XILINX_DMA_DMACR_RUNSTOP);
-}
-
-/**
- * xilinx_dma_is_idle - Check if DMA channel is idle
- * @chan: Driver specific DMA channel
- *
- * Return: '1' if idle, '0' if not.
- */
-static bool xilinx_dma_is_idle(struct xilinx_dma_chan *chan)
-{
-   return dma_ctrl_read(chan, XILINX_DMA_REG_DMASR) &
-   XILINX_DMA_DMASR_IDLE;
-}
-
-/**
  * xilinx_dma_halt - Halt DMA channel
  * @chan: Driver specific DMA channel
  */
@@ -966,6 +942,7 @@ static void xilinx_dma_halt(struct xilinx_dma_chan *chan)
chan, dma_ctrl_read(chan, XILINX_DMA_REG_DMASR));
chan->err = true;
}
+   chan->idle = true;
 }
 
 /**
@@ -1007,6 +984,9 @@ static void xilinx_vdma_start_transfer(struct 
xilinx_dma_chan *chan)
if (chan->err)
return;
 
+   if (!chan->idle)
+   return;
+
if (list_empty(>pending_list))
return;
 
@@ -1018,13 +998,6 @@ static void xilinx_vdma_start_transfer(struct 
xilinx_dma_chan *chan)
tail_segment = list_last_entry(_desc->segments,
   struct xilinx_vdma_tx_segment, node);
 
-   /* If it is SG mode and hardware is busy, cannot submit */
-   if (chan->has_sg && xilinx_dma_is_running(chan) &&
-   !xilinx_dma_is_idle(chan)) {
-   dev_dbg(chan->dev, "DMA controller still busy\n");
-   return;
-   }
-
/*
 * If hardware is idle, then all descriptors on the running lists are
 * done, start new transfers
@@ -1110,6 +1083,7 @@ static void xilinx_vdma_start_transfer(struct 
xilinx_dma_chan *chan)
vdma_desc_write(chan, XILINX_DMA_REG_VSIZE, last->hw.vsize);
}
 
+   chan->idle = false;
if (!chan->has_sg) {
list_del(>node);
list_add_tail(>node, >active_list);
@@ -1136,6 +1110,9 @@ static void xilinx_cdma_start_transfer(struct 
xilinx_dma_chan *chan)
if (chan->err)
return;
 
+   if (!chan->idle)
+   return;
+
if (list_empty(>pending_list))
return;
 
@@ -1181,6 +1158,7 @@ static void xilinx_cdma_start_transfer(struct 
xilinx_dma_chan *chan)
 
list_splice_tail_init(>pending_list, >active_list);
chan->desc_pendingcount = 0;
+   chan->idle = false;
 }
 
 /**
@@ -1196,15 +1174,11 @@ static void xilinx_dma_start_transfer(struct 
xilinx_dma_chan *chan)
if (chan->err)
return;
 
-   if (list_empty(>pending_list))
+   if (!chan->idle)
return;
 
-   /* If it is SG mode and hardware is busy, cannot submit */
-   if (chan->has_sg && xilinx_dma_is_running(chan) &&
-   !xilinx_dma_is_idle(chan)) {
-   dev_dbg(chan->dev, "DMA controller still busy\n");
+   if (list_empty(>pending_list))
return;
-   }
 
head_desc = list_first_entry(>pending_list,
 struct xilinx_dma_tx_descriptor, node);
@@ -1302,6 +1276,7 @@ static void xilinx_dma_start_transfer(struct 
xilinx_dma_chan *chan)
 

[PATCH v3 1/3] dmaengine: xilinx_dma: Check for channel idle state before submitting dma descriptor

2017-01-03 Thread Kedareswara rao Appana
Add channel idle state to ensure that dma descriptor is not
submitted when VDMA engine is in progress.

Reviewed-by: Jose Abreu 
Signed-off-by: Kedareswara rao Appana 
---
Changes for v3:
---> None.
Changes for v2:
---> Add idle check in the reset as suggested by Jose Abreu
---> Removed xilinx_dma_is_running/xilinx_dma_is_idle checks
in the driver and used common idle checks across the driver
as suggested by Laurent Pinchart.

 drivers/dma/xilinx/xilinx_dma.c | 56 +
 1 file changed, 17 insertions(+), 39 deletions(-)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 8288fe4..be7eb41 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -321,6 +321,7 @@ struct xilinx_dma_tx_descriptor {
  * @cyclic: Check for cyclic transfers.
  * @genlock: Support genlock mode
  * @err: Channel has errors
+ * @idle: Check for channel idle
  * @tasklet: Cleanup work after irq
  * @config: Device configuration info
  * @flush_on_fsync: Flush on Frame sync
@@ -351,6 +352,7 @@ struct xilinx_dma_chan {
bool cyclic;
bool genlock;
bool err;
+   bool idle;
struct tasklet_struct tasklet;
struct xilinx_vdma_config config;
bool flush_on_fsync;
@@ -920,32 +922,6 @@ static enum dma_status xilinx_dma_tx_status(struct 
dma_chan *dchan,
 }
 
 /**
- * xilinx_dma_is_running - Check if DMA channel is running
- * @chan: Driver specific DMA channel
- *
- * Return: '1' if running, '0' if not.
- */
-static bool xilinx_dma_is_running(struct xilinx_dma_chan *chan)
-{
-   return !(dma_ctrl_read(chan, XILINX_DMA_REG_DMASR) &
-XILINX_DMA_DMASR_HALTED) &&
-   (dma_ctrl_read(chan, XILINX_DMA_REG_DMACR) &
-XILINX_DMA_DMACR_RUNSTOP);
-}
-
-/**
- * xilinx_dma_is_idle - Check if DMA channel is idle
- * @chan: Driver specific DMA channel
- *
- * Return: '1' if idle, '0' if not.
- */
-static bool xilinx_dma_is_idle(struct xilinx_dma_chan *chan)
-{
-   return dma_ctrl_read(chan, XILINX_DMA_REG_DMASR) &
-   XILINX_DMA_DMASR_IDLE;
-}
-
-/**
  * xilinx_dma_halt - Halt DMA channel
  * @chan: Driver specific DMA channel
  */
@@ -966,6 +942,7 @@ static void xilinx_dma_halt(struct xilinx_dma_chan *chan)
chan, dma_ctrl_read(chan, XILINX_DMA_REG_DMASR));
chan->err = true;
}
+   chan->idle = true;
 }
 
 /**
@@ -1007,6 +984,9 @@ static void xilinx_vdma_start_transfer(struct 
xilinx_dma_chan *chan)
if (chan->err)
return;
 
+   if (!chan->idle)
+   return;
+
if (list_empty(>pending_list))
return;
 
@@ -1018,13 +998,6 @@ static void xilinx_vdma_start_transfer(struct 
xilinx_dma_chan *chan)
tail_segment = list_last_entry(_desc->segments,
   struct xilinx_vdma_tx_segment, node);
 
-   /* If it is SG mode and hardware is busy, cannot submit */
-   if (chan->has_sg && xilinx_dma_is_running(chan) &&
-   !xilinx_dma_is_idle(chan)) {
-   dev_dbg(chan->dev, "DMA controller still busy\n");
-   return;
-   }
-
/*
 * If hardware is idle, then all descriptors on the running lists are
 * done, start new transfers
@@ -1110,6 +1083,7 @@ static void xilinx_vdma_start_transfer(struct 
xilinx_dma_chan *chan)
vdma_desc_write(chan, XILINX_DMA_REG_VSIZE, last->hw.vsize);
}
 
+   chan->idle = false;
if (!chan->has_sg) {
list_del(>node);
list_add_tail(>node, >active_list);
@@ -1136,6 +1110,9 @@ static void xilinx_cdma_start_transfer(struct 
xilinx_dma_chan *chan)
if (chan->err)
return;
 
+   if (!chan->idle)
+   return;
+
if (list_empty(>pending_list))
return;
 
@@ -1181,6 +1158,7 @@ static void xilinx_cdma_start_transfer(struct 
xilinx_dma_chan *chan)
 
list_splice_tail_init(>pending_list, >active_list);
chan->desc_pendingcount = 0;
+   chan->idle = false;
 }
 
 /**
@@ -1196,15 +1174,11 @@ static void xilinx_dma_start_transfer(struct 
xilinx_dma_chan *chan)
if (chan->err)
return;
 
-   if (list_empty(>pending_list))
+   if (!chan->idle)
return;
 
-   /* If it is SG mode and hardware is busy, cannot submit */
-   if (chan->has_sg && xilinx_dma_is_running(chan) &&
-   !xilinx_dma_is_idle(chan)) {
-   dev_dbg(chan->dev, "DMA controller still busy\n");
+   if (list_empty(>pending_list))
return;
-   }
 
head_desc = list_first_entry(>pending_list,
 struct xilinx_dma_tx_descriptor, node);
@@ -1302,6 +1276,7 @@ static void xilinx_dma_start_transfer(struct 
xilinx_dma_chan *chan)
 
list_splice_tail_init(>pending_list, >active_list);

[RESEND,PATCH v7 2/2] Documentation/ABI: Add ABI information for QCOM socinfo driver

2017-01-03 Thread Imran Khan
The socinfo ABI document describes the information provided
by socinfo driver and the corresponding attributes to access
that information.

Signed-off-by: Imran Khan 
---
 Documentation/ABI/stable/sysfs-driver-qcom_socinfo | 147 +
 1 file changed, 147 insertions(+)
 create mode 100644 Documentation/ABI/stable/sysfs-driver-qcom_socinfo

diff --git a/Documentation/ABI/stable/sysfs-driver-qcom_socinfo 
b/Documentation/ABI/stable/sysfs-driver-qcom_socinfo
new file mode 100644
index 000..f90c142
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-driver-qcom_socinfo
@@ -0,0 +1,147 @@
+What:  /sys/devices/soc0/accessory_chip
+Date:  January 2017
+Description:
+   This file shows the id of the accessory chip.
+
+What:  /sys/devices/soc0/adsp_image_crm
+What:  /sys/devices/soc0/adsp_image_variant
+What:  /sys/devices/soc0/adsp_image_version
+Date:  January 2017
+Description:
+   These files respectively show the crm version, variant and
+   version of the ADSP image.
+
+What:  /sys/devices/soc0/apps_image_crm
+What:  /sys/devices/soc0/apps_image_variant
+What:  /sys/devices/soc0/apps_image_version
+Date:  January 2017
+Description:
+   These files respectively show the crm version, variant and
+   version of the APPS(Linux kernel, rootfs) image.
+
+What:  /sys/devices/soc0/boot_image_crm
+What:  /sys/devices/soc0/boot_image_variant
+What:  /sys/devices/soc0/boot_image_version
+Date:  January 2017
+Description:
+   These files respectively show the crm version, variant and
+   version of the Boot(bootloader) image.
+
+What:  /sys/devices/soc0/build_id
+Date:  January 2017
+Description:
+   This file shows the unique id of current build being used on
+   the system.
+
+What:  /sys/devices/soc0/cnss_image_crm
+What:  /sys/devices/soc0/cnss_image_variant
+What:  /sys/devices/soc0/cnss_image_version
+Date:  January 2017
+Description:
+   These files respectively show the crm version, variant and
+   version of the CNSS image.
+
+What:  /sys/devices/soc0/family
+Date:  January 2017
+Description:
+   This file shows the family(e.g Snapdragon) of the SoC.
+
+What:  /sys/devices/soc0/foundry_id
+Date:  January 2017
+Description:
+   This file shows the id of the foundry, where soc was
+   manufactured.
+
+What:  /sys/devices/soc0/hw_platform
+Date:  January 2017
+Description:
+   This file shows the type of hardware platform
+   (e.g MTP, QRD etc) where SoC is being used.
+
+What:  /sys/devices/soc0/machine
+Date:  January 2017
+Description:
+   This file shows the machine name as given in the DT.
+
+What:  /sys/devices/soc0/mpss_image_crm
+What:  /sys/devices/soc0/mpss_image_variant
+What:  /sys/devices/soc0/mpss_image_version
+Date:  January 2017
+Description:
+   These files respectively show the crm version, variant and
+   version of the MPSS image.
+
+What:  /sys/devices/soc0/platform_subtype
+Date:  January 2017
+Description:
+   This file shows the sub-type of hardware platform
+   (SKUAA, SKUF etc.) where SoC is being used.
+
+What:  /sys/devices/soc0/platform_version
+Date:  January 2017
+Description:
+   This file show the version of the hardware platform.
+
+What:  /sys/devices/soc0/pmic_die_revision
+Date:  January 2017
+Description:
+   This file shows revision of PMIC die.
+
+What:  /sys/devices/soc0/pmic_model
+Date:  January 2017
+Description:
+   This file shows name of PMIC model.
+
+What:  /sys/devices/soc0/qcom_odm
+Date:  January 2017
+Description:
+   This file shows the ODM using the SoC.
+
+What:  /sys/devices/soc0/raw_version
+Date:  January 2017
+Description:
+   This file shows raw version of the SoC.
+
+What:  /sys/devices/soc0/revision
+Date:  January 2017
+Description:
+   This file shows revision of the SoC.
+
+What:  /sys/devices/soc0/rpm_image_crm
+What:  /sys/devices/soc0/rpm_image_variant
+What:  /sys/devices/soc0/rpm_image_version
+Date:  January 2017
+Description:
+   These files respectively show the crm version, variant and
+   version of the RPM image.
+
+What:  /sys/devices/soc0/serial_number
+Date:  January 2017
+Description:
+   This file shows serial number of the SoC.
+
+What:  /sys/devices/soc0/soc_id
+Date:  January 2017
+Description:
+   

[RESEND,PATCH v7 0/2] soc: qcom: Add SoC info driver

2017-01-03 Thread Imran Khan
This is patchset v7 which takes care of review comments
received for patchset v6. Resending the patch set as
the last one had a compilation error. 

Major changes with respect to patch v6 are as below:
 - Rather than showing string for h/w types, just show the
   number obtained from SMEM. The reader of this number
   is supposed to parse this into a human readable string 
   as this information may be different for different ODMs
 - Create an attribute 'qcom_odm' depending on the presence
   of corresponding property in SMEM DT entry. This information
   can be used for ODM specific interpretation of SMEM socinfo
   content
 - Read machine value from DT, rather than using soc-id as index
   in an ever increasing array of machine names

Imran Khan (2):
  soc: qcom: Add SoC info driver
  Documentation/ABI: Add ABI information for QCOM socinfo driver

 Documentation/ABI/stable/sysfs-driver-qcom_socinfo | 147 ++
 drivers/soc/qcom/Kconfig   |   1 +
 drivers/soc/qcom/Makefile  |   3 +-
 drivers/soc/qcom/smem.c|   5 +
 drivers/soc/qcom/socinfo.c | 515 +
 5 files changed, 670 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/stable/sysfs-driver-qcom_socinfo
 create mode 100644 drivers/soc/qcom/socinfo.c

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[RESEND,PATCH v7 2/2] Documentation/ABI: Add ABI information for QCOM socinfo driver

2017-01-03 Thread Imran Khan
The socinfo ABI document describes the information provided
by socinfo driver and the corresponding attributes to access
that information.

Signed-off-by: Imran Khan 
---
 Documentation/ABI/stable/sysfs-driver-qcom_socinfo | 147 +
 1 file changed, 147 insertions(+)
 create mode 100644 Documentation/ABI/stable/sysfs-driver-qcom_socinfo

diff --git a/Documentation/ABI/stable/sysfs-driver-qcom_socinfo 
b/Documentation/ABI/stable/sysfs-driver-qcom_socinfo
new file mode 100644
index 000..f90c142
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-driver-qcom_socinfo
@@ -0,0 +1,147 @@
+What:  /sys/devices/soc0/accessory_chip
+Date:  January 2017
+Description:
+   This file shows the id of the accessory chip.
+
+What:  /sys/devices/soc0/adsp_image_crm
+What:  /sys/devices/soc0/adsp_image_variant
+What:  /sys/devices/soc0/adsp_image_version
+Date:  January 2017
+Description:
+   These files respectively show the crm version, variant and
+   version of the ADSP image.
+
+What:  /sys/devices/soc0/apps_image_crm
+What:  /sys/devices/soc0/apps_image_variant
+What:  /sys/devices/soc0/apps_image_version
+Date:  January 2017
+Description:
+   These files respectively show the crm version, variant and
+   version of the APPS(Linux kernel, rootfs) image.
+
+What:  /sys/devices/soc0/boot_image_crm
+What:  /sys/devices/soc0/boot_image_variant
+What:  /sys/devices/soc0/boot_image_version
+Date:  January 2017
+Description:
+   These files respectively show the crm version, variant and
+   version of the Boot(bootloader) image.
+
+What:  /sys/devices/soc0/build_id
+Date:  January 2017
+Description:
+   This file shows the unique id of current build being used on
+   the system.
+
+What:  /sys/devices/soc0/cnss_image_crm
+What:  /sys/devices/soc0/cnss_image_variant
+What:  /sys/devices/soc0/cnss_image_version
+Date:  January 2017
+Description:
+   These files respectively show the crm version, variant and
+   version of the CNSS image.
+
+What:  /sys/devices/soc0/family
+Date:  January 2017
+Description:
+   This file shows the family(e.g Snapdragon) of the SoC.
+
+What:  /sys/devices/soc0/foundry_id
+Date:  January 2017
+Description:
+   This file shows the id of the foundry, where soc was
+   manufactured.
+
+What:  /sys/devices/soc0/hw_platform
+Date:  January 2017
+Description:
+   This file shows the type of hardware platform
+   (e.g MTP, QRD etc) where SoC is being used.
+
+What:  /sys/devices/soc0/machine
+Date:  January 2017
+Description:
+   This file shows the machine name as given in the DT.
+
+What:  /sys/devices/soc0/mpss_image_crm
+What:  /sys/devices/soc0/mpss_image_variant
+What:  /sys/devices/soc0/mpss_image_version
+Date:  January 2017
+Description:
+   These files respectively show the crm version, variant and
+   version of the MPSS image.
+
+What:  /sys/devices/soc0/platform_subtype
+Date:  January 2017
+Description:
+   This file shows the sub-type of hardware platform
+   (SKUAA, SKUF etc.) where SoC is being used.
+
+What:  /sys/devices/soc0/platform_version
+Date:  January 2017
+Description:
+   This file show the version of the hardware platform.
+
+What:  /sys/devices/soc0/pmic_die_revision
+Date:  January 2017
+Description:
+   This file shows revision of PMIC die.
+
+What:  /sys/devices/soc0/pmic_model
+Date:  January 2017
+Description:
+   This file shows name of PMIC model.
+
+What:  /sys/devices/soc0/qcom_odm
+Date:  January 2017
+Description:
+   This file shows the ODM using the SoC.
+
+What:  /sys/devices/soc0/raw_version
+Date:  January 2017
+Description:
+   This file shows raw version of the SoC.
+
+What:  /sys/devices/soc0/revision
+Date:  January 2017
+Description:
+   This file shows revision of the SoC.
+
+What:  /sys/devices/soc0/rpm_image_crm
+What:  /sys/devices/soc0/rpm_image_variant
+What:  /sys/devices/soc0/rpm_image_version
+Date:  January 2017
+Description:
+   These files respectively show the crm version, variant and
+   version of the RPM image.
+
+What:  /sys/devices/soc0/serial_number
+Date:  January 2017
+Description:
+   This file shows serial number of the SoC.
+
+What:  /sys/devices/soc0/soc_id
+Date:  January 2017
+Description:
+   This file shows the 

[RESEND,PATCH v7 0/2] soc: qcom: Add SoC info driver

2017-01-03 Thread Imran Khan
This is patchset v7 which takes care of review comments
received for patchset v6. Resending the patch set as
the last one had a compilation error. 

Major changes with respect to patch v6 are as below:
 - Rather than showing string for h/w types, just show the
   number obtained from SMEM. The reader of this number
   is supposed to parse this into a human readable string 
   as this information may be different for different ODMs
 - Create an attribute 'qcom_odm' depending on the presence
   of corresponding property in SMEM DT entry. This information
   can be used for ODM specific interpretation of SMEM socinfo
   content
 - Read machine value from DT, rather than using soc-id as index
   in an ever increasing array of machine names

Imran Khan (2):
  soc: qcom: Add SoC info driver
  Documentation/ABI: Add ABI information for QCOM socinfo driver

 Documentation/ABI/stable/sysfs-driver-qcom_socinfo | 147 ++
 drivers/soc/qcom/Kconfig   |   1 +
 drivers/soc/qcom/Makefile  |   3 +-
 drivers/soc/qcom/smem.c|   5 +
 drivers/soc/qcom/socinfo.c | 515 +
 5 files changed, 670 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/stable/sysfs-driver-qcom_socinfo
 create mode 100644 drivers/soc/qcom/socinfo.c

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v2 1/1] extcon: usb-gpio: add pinctrl operation during system PM

2017-01-03 Thread Peter Chen
At some systems, the pinctrl setting will be lost or needs to
set as "sleep" state to save power consumption. So, we need to
configure pinctrl as "sleep" state when system enters suspend,
and as "default" state after system resumes. In this way, the
pinctrl value can be recovered as "default" state after resuming.

Signed-off-by: Peter Chen 
---
Changes for v2:
- Add header file for pinctrl to avoid build error
- Only set pin as "sleep" state when the wakeup is not requested

 drivers/extcon/extcon-usb-gpio.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
index d589c5f..a5e1882 100644
--- a/drivers/extcon/extcon-usb-gpio.c
+++ b/drivers/extcon/extcon-usb-gpio.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define USB_GPIO_DEBOUNCE_MS   20  /* ms */
 
@@ -245,6 +246,9 @@ static int usb_extcon_suspend(struct device *dev)
if (info->vbus_gpiod)
disable_irq(info->vbus_irq);
 
+   if (!device_may_wakeup(dev))
+   pinctrl_pm_select_sleep_state(dev);
+
return ret;
 }
 
@@ -253,6 +257,9 @@ static int usb_extcon_resume(struct device *dev)
struct usb_extcon_info *info = dev_get_drvdata(dev);
int ret = 0;
 
+   if (!device_may_wakeup(dev))
+   pinctrl_pm_select_default_state(dev);
+
if (device_may_wakeup(dev)) {
if (info->id_gpiod) {
ret = disable_irq_wake(info->id_irq);
-- 
2.7.4



[PATCH v2 1/1] extcon: usb-gpio: add pinctrl operation during system PM

2017-01-03 Thread Peter Chen
At some systems, the pinctrl setting will be lost or needs to
set as "sleep" state to save power consumption. So, we need to
configure pinctrl as "sleep" state when system enters suspend,
and as "default" state after system resumes. In this way, the
pinctrl value can be recovered as "default" state after resuming.

Signed-off-by: Peter Chen 
---
Changes for v2:
- Add header file for pinctrl to avoid build error
- Only set pin as "sleep" state when the wakeup is not requested

 drivers/extcon/extcon-usb-gpio.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
index d589c5f..a5e1882 100644
--- a/drivers/extcon/extcon-usb-gpio.c
+++ b/drivers/extcon/extcon-usb-gpio.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define USB_GPIO_DEBOUNCE_MS   20  /* ms */
 
@@ -245,6 +246,9 @@ static int usb_extcon_suspend(struct device *dev)
if (info->vbus_gpiod)
disable_irq(info->vbus_irq);
 
+   if (!device_may_wakeup(dev))
+   pinctrl_pm_select_sleep_state(dev);
+
return ret;
 }
 
@@ -253,6 +257,9 @@ static int usb_extcon_resume(struct device *dev)
struct usb_extcon_info *info = dev_get_drvdata(dev);
int ret = 0;
 
+   if (!device_may_wakeup(dev))
+   pinctrl_pm_select_default_state(dev);
+
if (device_may_wakeup(dev)) {
if (info->id_gpiod) {
ret = disable_irq_wake(info->id_irq);
-- 
2.7.4



Re: [PATCH] dax: fix deadlock with DAX 4k holes

2017-01-03 Thread Jan Kara
On Tue 03-01-17 14:36:05, Ross Zwisler wrote:
> Currently in DAX if we have three read faults on the same hole address we
> can end up with the following:
> 
> Thread 0  Thread 1Thread 2
>   
> dax_iomap_fault
>  grab_mapping_entry
>   lock_slot
>
> 
>   dax_iomap_fault
>grab_mapping_entry
> get_unlocked_mapping_entry
>  
> 
>   dax_iomap_fault
>grab_mapping_entry
> get_unlocked_mapping_entry
>  
>   dax_load_hole
>find_or_create_page
>...
> page_cache_tree_insert
>  dax_wake_mapping_entry_waiter
>   
>  __radix_tree_replace
>   
> 
>   
>   get_page
>   lock_page
>   ...
>   put_locked_mapping_entry
>   unlock_page
>   put_page
> 
>   wait queue>
> 
> The crux of the problem is that once we insert a 4k zero page, all locking
> from then on is done in terms of that 4k zero page and any additional
> threads sleeping on the empty DAX entry will never be woken.  Fix this by
> waking all sleepers when we replace the DAX radix tree entry with a 4k zero
> page.  This will allow all sleeping threads to successfully transition from
> locking based on the DAX empty entry to locking on the 4k zero page.
> 
> With the test case reported by Xiong this happens very regularly in my test
> setup, with some runs resulting in 9+ threads in this deadlocked state.
> With this fix I've been able to run that same test dozens of times in a
> loop without issue.
> 
> Signed-off-by: Ross Zwisler 
> Reported-by: Xiong Zhou 
> Fixes: commit ac401cc78242 ("dax: New fault locking")
> Cc: Jan Kara 
> Cc: sta...@vger.kernel.org # 4.7+

Ah, very good catch. You can add:

Reviewed-by: Jan Kara 

I wonder why I was not able to reproduce this... Probably the timing didn't
work out right on my test machine.

Honza

> ---
> 
> This issue exists as far back as v4.7, and I was easly able to reproduce it
> with v4.7 using the same test.
> 
> Unfortunately this patch won't apply cleanly to the stable trees, but the
> change is very simple and should be easy to replicate by hand.  Please ping
> me if you'd like patches that apply cleanly to the v4.9 and v4.8.15 trees.
> 
> ---
>  mm/filemap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/filemap.c b/mm/filemap.c
> index d0e4d10..b772a33 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -138,7 +138,7 @@ static int page_cache_tree_insert(struct address_space 
> *mapping,
>   dax_radix_locked_entry(0, RADIX_DAX_EMPTY));
>   /* Wakeup waiters for exceptional entry lock */
>   dax_wake_mapping_entry_waiter(mapping, page->index, p,
> -   false);
> +   true);
>   }
>   }
>   __radix_tree_replace(>page_tree, node, slot, page,
> -- 
> 2.7.4
> 
-- 
Jan Kara 
SUSE Labs, CR


Re: [PATCH] dax: fix deadlock with DAX 4k holes

2017-01-03 Thread Jan Kara
On Tue 03-01-17 14:36:05, Ross Zwisler wrote:
> Currently in DAX if we have three read faults on the same hole address we
> can end up with the following:
> 
> Thread 0  Thread 1Thread 2
>   
> dax_iomap_fault
>  grab_mapping_entry
>   lock_slot
>
> 
>   dax_iomap_fault
>grab_mapping_entry
> get_unlocked_mapping_entry
>  
> 
>   dax_iomap_fault
>grab_mapping_entry
> get_unlocked_mapping_entry
>  
>   dax_load_hole
>find_or_create_page
>...
> page_cache_tree_insert
>  dax_wake_mapping_entry_waiter
>   
>  __radix_tree_replace
>   
> 
>   
>   get_page
>   lock_page
>   ...
>   put_locked_mapping_entry
>   unlock_page
>   put_page
> 
>   wait queue>
> 
> The crux of the problem is that once we insert a 4k zero page, all locking
> from then on is done in terms of that 4k zero page and any additional
> threads sleeping on the empty DAX entry will never be woken.  Fix this by
> waking all sleepers when we replace the DAX radix tree entry with a 4k zero
> page.  This will allow all sleeping threads to successfully transition from
> locking based on the DAX empty entry to locking on the 4k zero page.
> 
> With the test case reported by Xiong this happens very regularly in my test
> setup, with some runs resulting in 9+ threads in this deadlocked state.
> With this fix I've been able to run that same test dozens of times in a
> loop without issue.
> 
> Signed-off-by: Ross Zwisler 
> Reported-by: Xiong Zhou 
> Fixes: commit ac401cc78242 ("dax: New fault locking")
> Cc: Jan Kara 
> Cc: sta...@vger.kernel.org # 4.7+

Ah, very good catch. You can add:

Reviewed-by: Jan Kara 

I wonder why I was not able to reproduce this... Probably the timing didn't
work out right on my test machine.

Honza

> ---
> 
> This issue exists as far back as v4.7, and I was easly able to reproduce it
> with v4.7 using the same test.
> 
> Unfortunately this patch won't apply cleanly to the stable trees, but the
> change is very simple and should be easy to replicate by hand.  Please ping
> me if you'd like patches that apply cleanly to the v4.9 and v4.8.15 trees.
> 
> ---
>  mm/filemap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/filemap.c b/mm/filemap.c
> index d0e4d10..b772a33 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -138,7 +138,7 @@ static int page_cache_tree_insert(struct address_space 
> *mapping,
>   dax_radix_locked_entry(0, RADIX_DAX_EMPTY));
>   /* Wakeup waiters for exceptional entry lock */
>   dax_wake_mapping_entry_waiter(mapping, page->index, p,
> -   false);
> +   true);
>   }
>   }
>   __radix_tree_replace(>page_tree, node, slot, page,
> -- 
> 2.7.4
> 
-- 
Jan Kara 
SUSE Labs, CR


[RESEND,PATCH v7 1/2] soc: qcom: Add SoC info driver

2017-01-03 Thread Imran Khan
The SoC info driver provides information such as Chip ID,
Chip family, serial number and other such details about
Qualcomm SoCs to user space, so that if needed some user
space utility(like antutu) can query such information
using sysfs interface.

Signed-off-by: Imran Khan 
---
v6 --> v7:
 - Some minor style changes
 - Rather than showing string for h/w types, just show the
   number obtained from SMEM. The reader of this number
   is supposed to parse this into a human readable string 
   as this information may be different for different ODMs
 - Create an attribute 'qcom_odm' depending on the presence
   of corresponding property in SMEM DT entry. This information
   can be used for ODM specific interpretation of SMEM socinfo
   content
 - Read machine value from DT, rather than using soc-id as index
   in an ever increasing array of machine names

v5 --> v6:
 - use dev_ext_attribute to represent SMEM image items
 - Avoid redundant function calls
 - Avoid use of unnecessary global variables
 - Remove redundant enums
 - Avoid redundant checking of socinfo being NULL or not
 - Avoid redundant checking of socinfo format version
 - Rename show/store function of attributes as _show/_store
   rather than _get/_set
 - Use an array to represent socinfo attributes and create
   attribute files in a loop depending on the minimum 
   socinfo format version for which these attributes are 
   supported
 - if obtained socinfo format version is greater than the
   maximum known version return an error rather than falling
   back to maximum known version
 
v4 --> v5:
 - Removed redundant function socinfo_print

v3 --> v4:
 - Corrected makefile so that smem and socinfo are treated as one module
 - Moved the code snippet to get socinfo smem item into socinfo.c
 - Removed redundant use of socinfo major version as it is always zero
 - Removed unused enums
 - Removed redundant indirections
 - Use image_version object to store information about each entry in the
   smem image table
 - Replaced usage of snprintf with sprintf and scnprintf
 - Get the address of image version table at the beginning and setup
   image version attributes only if image version table is available
 - Do the same for platform_subtype
 - Make different type of image version objects read only or readable/
   writable depending on their types. For example apps image attributes
   can be modified via sysfs but the same can't be done for modem image
 - Show PMIC model in a human readable form rather than a numeric number
 - Avoid using table in single sysfs entry
 - Removed checkpatch warnings about S_IRUGO

v2 --> v3:
 - Add support to toss soc information data into entropy pool
 - Since socinfo is rolled into smem driver, compile the
   relevant portion of socinfo driver with smem driver
 
v1 --> v2:
 - Removed inclusion of system_misc.h
 - merged socinfo.h into socinfo.c
 - made platform type and subtype arrays static
 - replaced uint32_t with u32
 - made functions static to avoid exposing vendor specific interface
 - Replaced usage of IS_ERR_OR_NULL with IS_ERR.
 - Remove raw-id attribute usage as human readable soc-id will suffice here
 - Avoid using a separate platform driver for socinfo by rolling it into smem 
driver itself.
   The sysfs setup is being done in a separate file (socinfo.c)
 - Replaced macro BUILD_ID_LENGTH with  SMEM_SOCINFO_BUILD_ID_LENGTH.
 - For failure cases where socinfo can not be read use a single dummy socinfo 
with error values.
 - Removed usage of early_machine_is_xxx.


 drivers/soc/qcom/Kconfig   |   1 +
 drivers/soc/qcom/Makefile  |   3 +-
 drivers/soc/qcom/smem.c|   5 +
 drivers/soc/qcom/socinfo.c | 515 +
 4 files changed, 523 insertions(+), 1 deletion(-)
 create mode 100644 drivers/soc/qcom/socinfo.c

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 461b387..f89d34d 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -24,6 +24,7 @@ config QCOM_SMEM
tristate "Qualcomm Shared Memory Manager (SMEM)"
depends on ARCH_QCOM
depends on HWSPINLOCK
+   select SOC_BUS
help
  Say y here to enable support for the Qualcomm Shared Memory Manager.
  The driver provides an interface to items in a heap shared among all
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index fdd664e..438efec 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -2,7 +2,8 @@ obj-$(CONFIG_QCOM_GSBI) +=  qcom_gsbi.o
 obj-$(CONFIG_QCOM_PM)  +=  spm.o
 obj-$(CONFIG_QCOM_SMD) +=  smd.o
 obj-$(CONFIG_QCOM_SMD_RPM) += smd-rpm.o
-obj-$(CONFIG_QCOM_SMEM) += smem.o
+obj-$(CONFIG_QCOM_SMEM) += qcom_smem.o
+qcom_smem-y := smem.o socinfo.o
 obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
 obj-$(CONFIG_QCOM_SMP2P)   += smp2p.o
 obj-$(CONFIG_QCOM_SMSM)+= smsm.o
diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
index 

[RESEND,PATCH v7 1/2] soc: qcom: Add SoC info driver

2017-01-03 Thread Imran Khan
The SoC info driver provides information such as Chip ID,
Chip family, serial number and other such details about
Qualcomm SoCs to user space, so that if needed some user
space utility(like antutu) can query such information
using sysfs interface.

Signed-off-by: Imran Khan 
---
v6 --> v7:
 - Some minor style changes
 - Rather than showing string for h/w types, just show the
   number obtained from SMEM. The reader of this number
   is supposed to parse this into a human readable string 
   as this information may be different for different ODMs
 - Create an attribute 'qcom_odm' depending on the presence
   of corresponding property in SMEM DT entry. This information
   can be used for ODM specific interpretation of SMEM socinfo
   content
 - Read machine value from DT, rather than using soc-id as index
   in an ever increasing array of machine names

v5 --> v6:
 - use dev_ext_attribute to represent SMEM image items
 - Avoid redundant function calls
 - Avoid use of unnecessary global variables
 - Remove redundant enums
 - Avoid redundant checking of socinfo being NULL or not
 - Avoid redundant checking of socinfo format version
 - Rename show/store function of attributes as _show/_store
   rather than _get/_set
 - Use an array to represent socinfo attributes and create
   attribute files in a loop depending on the minimum 
   socinfo format version for which these attributes are 
   supported
 - if obtained socinfo format version is greater than the
   maximum known version return an error rather than falling
   back to maximum known version
 
v4 --> v5:
 - Removed redundant function socinfo_print

v3 --> v4:
 - Corrected makefile so that smem and socinfo are treated as one module
 - Moved the code snippet to get socinfo smem item into socinfo.c
 - Removed redundant use of socinfo major version as it is always zero
 - Removed unused enums
 - Removed redundant indirections
 - Use image_version object to store information about each entry in the
   smem image table
 - Replaced usage of snprintf with sprintf and scnprintf
 - Get the address of image version table at the beginning and setup
   image version attributes only if image version table is available
 - Do the same for platform_subtype
 - Make different type of image version objects read only or readable/
   writable depending on their types. For example apps image attributes
   can be modified via sysfs but the same can't be done for modem image
 - Show PMIC model in a human readable form rather than a numeric number
 - Avoid using table in single sysfs entry
 - Removed checkpatch warnings about S_IRUGO

v2 --> v3:
 - Add support to toss soc information data into entropy pool
 - Since socinfo is rolled into smem driver, compile the
   relevant portion of socinfo driver with smem driver
 
v1 --> v2:
 - Removed inclusion of system_misc.h
 - merged socinfo.h into socinfo.c
 - made platform type and subtype arrays static
 - replaced uint32_t with u32
 - made functions static to avoid exposing vendor specific interface
 - Replaced usage of IS_ERR_OR_NULL with IS_ERR.
 - Remove raw-id attribute usage as human readable soc-id will suffice here
 - Avoid using a separate platform driver for socinfo by rolling it into smem 
driver itself.
   The sysfs setup is being done in a separate file (socinfo.c)
 - Replaced macro BUILD_ID_LENGTH with  SMEM_SOCINFO_BUILD_ID_LENGTH.
 - For failure cases where socinfo can not be read use a single dummy socinfo 
with error values.
 - Removed usage of early_machine_is_xxx.


 drivers/soc/qcom/Kconfig   |   1 +
 drivers/soc/qcom/Makefile  |   3 +-
 drivers/soc/qcom/smem.c|   5 +
 drivers/soc/qcom/socinfo.c | 515 +
 4 files changed, 523 insertions(+), 1 deletion(-)
 create mode 100644 drivers/soc/qcom/socinfo.c

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 461b387..f89d34d 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -24,6 +24,7 @@ config QCOM_SMEM
tristate "Qualcomm Shared Memory Manager (SMEM)"
depends on ARCH_QCOM
depends on HWSPINLOCK
+   select SOC_BUS
help
  Say y here to enable support for the Qualcomm Shared Memory Manager.
  The driver provides an interface to items in a heap shared among all
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index fdd664e..438efec 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -2,7 +2,8 @@ obj-$(CONFIG_QCOM_GSBI) +=  qcom_gsbi.o
 obj-$(CONFIG_QCOM_PM)  +=  spm.o
 obj-$(CONFIG_QCOM_SMD) +=  smd.o
 obj-$(CONFIG_QCOM_SMD_RPM) += smd-rpm.o
-obj-$(CONFIG_QCOM_SMEM) += smem.o
+obj-$(CONFIG_QCOM_SMEM) += qcom_smem.o
+qcom_smem-y := smem.o socinfo.o
 obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
 obj-$(CONFIG_QCOM_SMP2P)   += smp2p.o
 obj-$(CONFIG_QCOM_SMSM)+= smsm.o
diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
index 18ec52f..4f0ccf8 100644
--- 

[PATCH] memory: tegra: Add a missing 'of_node_put()' call

2017-01-03 Thread Christophe JAILLET
If 'of_find_device_by_node()' fails, an 'of_node_put()' call is missing in
the error handling path.
Fix it by reordering the code.

While at it, remove some empty lines in a more or less similar construction
a few lines below.

Signed-off-by: Christophe JAILLET 
---
 drivers/memory/tegra/tegra124-emc.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/memory/tegra/tegra124-emc.c 
b/drivers/memory/tegra/tegra124-emc.c
index 06cc781ebac1..392dc8dd481f 100644
--- a/drivers/memory/tegra/tegra124-emc.c
+++ b/drivers/memory/tegra/tegra124-emc.c
@@ -1115,11 +1115,10 @@ static int tegra_emc_probe(struct platform_device *pdev)
}
 
mc = of_find_device_by_node(np);
+   of_node_put(np);
if (!mc)
return -ENOENT;
 
-   of_node_put(np);
-
emc->mc = platform_get_drvdata(mc);
if (!emc->mc)
return -EPROBE_DEFER;
@@ -1135,9 +1134,7 @@ static int tegra_emc_probe(struct platform_device *pdev)
}
 
err = tegra_emc_load_timings_from_dt(emc, np);
-
of_node_put(np);
-
if (err)
return err;
 
-- 
2.9.3



[PATCH] memory: tegra: Add a missing 'of_node_put()' call

2017-01-03 Thread Christophe JAILLET
If 'of_find_device_by_node()' fails, an 'of_node_put()' call is missing in
the error handling path.
Fix it by reordering the code.

While at it, remove some empty lines in a more or less similar construction
a few lines below.

Signed-off-by: Christophe JAILLET 
---
 drivers/memory/tegra/tegra124-emc.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/memory/tegra/tegra124-emc.c 
b/drivers/memory/tegra/tegra124-emc.c
index 06cc781ebac1..392dc8dd481f 100644
--- a/drivers/memory/tegra/tegra124-emc.c
+++ b/drivers/memory/tegra/tegra124-emc.c
@@ -1115,11 +1115,10 @@ static int tegra_emc_probe(struct platform_device *pdev)
}
 
mc = of_find_device_by_node(np);
+   of_node_put(np);
if (!mc)
return -ENOENT;
 
-   of_node_put(np);
-
emc->mc = platform_get_drvdata(mc);
if (!emc->mc)
return -EPROBE_DEFER;
@@ -1135,9 +1134,7 @@ static int tegra_emc_probe(struct platform_device *pdev)
}
 
err = tegra_emc_load_timings_from_dt(emc, np);
-
of_node_put(np);
-
if (err)
return err;
 
-- 
2.9.3



Re: [PATCH v4 2/3] ARM: dts: imx6: Support Savageboard dual

2017-01-03 Thread Milo Kim


On 01/04/2017 01:55 PM, Milo Kim wrote:

Common savageboard DT file is used for board support.
Add the vendor name and specify the dtb file for i.MX6Q build.

Reviewed-by: Fabio Estevam 
Signed-off-by: Milo Kim 
---
 .../devicetree/bindings/vendor-prefixes.txt|  1 +
 arch/arm/boot/dts/Makefile |  1 +
 arch/arm/boot/dts/imx6dl-savageboard.dts   | 51 ++
 3 files changed, 53 insertions(+)
 create mode 100644 arch/arm/boot/dts/imx6dl-savageboard.dts

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt 
b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 16d3b5e7f5d1..88c33d827e51 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -227,6 +227,7 @@ pine64  Pine64
 pixcir  PIXCIR MICROELECTRONICS Co., Ltd
 plathome   Plat'Home Co., Ltd.
 plda   PLDA
+poslab  Poslab Technology Co., Ltd.


I should input tab here instead of spaces, so updated single patch was 
just sent. Please refer to the patch named "[PATCH resend v4 2/3] ARM: 
dts: imx6: Support Savageboard dual".


Sorry for the inconvenience.

Best regards,
Milo


Re: [PATCH v4 2/3] ARM: dts: imx6: Support Savageboard dual

2017-01-03 Thread Milo Kim


On 01/04/2017 01:55 PM, Milo Kim wrote:

Common savageboard DT file is used for board support.
Add the vendor name and specify the dtb file for i.MX6Q build.

Reviewed-by: Fabio Estevam 
Signed-off-by: Milo Kim 
---
 .../devicetree/bindings/vendor-prefixes.txt|  1 +
 arch/arm/boot/dts/Makefile |  1 +
 arch/arm/boot/dts/imx6dl-savageboard.dts   | 51 ++
 3 files changed, 53 insertions(+)
 create mode 100644 arch/arm/boot/dts/imx6dl-savageboard.dts

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt 
b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 16d3b5e7f5d1..88c33d827e51 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -227,6 +227,7 @@ pine64  Pine64
 pixcir  PIXCIR MICROELECTRONICS Co., Ltd
 plathome   Plat'Home Co., Ltd.
 plda   PLDA
+poslab  Poslab Technology Co., Ltd.


I should input tab here instead of spaces, so updated single patch was 
just sent. Please refer to the patch named "[PATCH resend v4 2/3] ARM: 
dts: imx6: Support Savageboard dual".


Sorry for the inconvenience.

Best regards,
Milo


[PATCH v3 3/3] dmaengine: xilinx_dma: Fix race condition in the driver for multiple descriptor scenario

2017-01-03 Thread Kedareswara rao Appana
When driver is handling AXI DMA SoftIP
When user submits multiple descriptors back to back on the S2MM(recv)
side with the current driver flow the last buffer descriptor next bd
points to a invalid location resulting the invalid data or errors in the
DMA engine.

This patch fixes this issue by creating a BD Chain during
channel allocation itself and use those BD's.

Signed-off-by: Kedareswara rao Appana 
---
Changes for v3:
---> None.
Changes for v2:
---> None.

 drivers/dma/xilinx/xilinx_dma.c | 133 +---
 1 file changed, 83 insertions(+), 50 deletions(-)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 7cd8e08..822ccf00 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -163,6 +163,7 @@
 #define XILINX_DMA_BD_SOP  BIT(27)
 #define XILINX_DMA_BD_EOP  BIT(26)
 #define XILINX_DMA_COALESCE_MAX255
+#define XILINX_DMA_NUM_DESCS   255
 #define XILINX_DMA_NUM_APP_WORDS   5
 
 /* Multi-Channel DMA Descriptor offsets*/
@@ -310,6 +311,7 @@ struct xilinx_dma_tx_descriptor {
  * @pending_list: Descriptors waiting
  * @active_list: Descriptors ready to submit
  * @done_list: Complete descriptors
+ * @free_seg_list: Free descriptors
  * @common: DMA common channel
  * @desc_pool: Descriptors pool
  * @dev: The dma device
@@ -331,7 +333,9 @@ struct xilinx_dma_tx_descriptor {
  * @desc_submitcount: Descriptor h/w submitted count
  * @residue: Residue for AXI DMA
  * @seg_v: Statically allocated segments base
+ * @seg_p: Physical allocated segments base
  * @cyclic_seg_v: Statically allocated segment base for cyclic transfers
+ * @cyclic_seg_p: Physical allocated segments base for cyclic dma
  * @start_transfer: Differentiate b/w DMA IP's transfer
  */
 struct xilinx_dma_chan {
@@ -342,6 +346,7 @@ struct xilinx_dma_chan {
struct list_head pending_list;
struct list_head active_list;
struct list_head done_list;
+   struct list_head free_seg_list;
struct dma_chan common;
struct dma_pool *desc_pool;
struct device *dev;
@@ -363,7 +368,9 @@ struct xilinx_dma_chan {
u32 desc_submitcount;
u32 residue;
struct xilinx_axidma_tx_segment *seg_v;
+   dma_addr_t seg_p;
struct xilinx_axidma_tx_segment *cyclic_seg_v;
+   dma_addr_t cyclic_seg_p;
void (*start_transfer)(struct xilinx_dma_chan *chan);
u16 tdest;
 };
@@ -569,17 +576,31 @@ static struct xilinx_axidma_tx_segment *
 xilinx_axidma_alloc_tx_segment(struct xilinx_dma_chan *chan)
 {
struct xilinx_axidma_tx_segment *segment;
-   dma_addr_t phys;
-
-   segment = dma_pool_zalloc(chan->desc_pool, GFP_ATOMIC, );
-   if (!segment)
-   return NULL;
+   unsigned long flags;
 
-   segment->phys = phys;
+   spin_lock_irqsave(>lock, flags);
+   if (!list_empty(>free_seg_list)) {
+   segment = list_first_entry(>free_seg_list,
+  struct xilinx_axidma_tx_segment,
+  node);
+   list_del(>node);
+   }
+   spin_unlock_irqrestore(>lock, flags);
 
return segment;
 }
 
+static void xilinx_dma_clean_hw_desc(struct xilinx_axidma_desc_hw *hw)
+{
+   u32 next_desc = hw->next_desc;
+   u32 next_desc_msb = hw->next_desc_msb;
+
+   memset(hw, 0, sizeof(struct xilinx_axidma_desc_hw));
+
+   hw->next_desc = next_desc;
+   hw->next_desc_msb = next_desc_msb;
+}
+
 /**
  * xilinx_dma_free_tx_segment - Free transaction segment
  * @chan: Driver specific DMA channel
@@ -588,7 +609,9 @@ xilinx_axidma_alloc_tx_segment(struct xilinx_dma_chan *chan)
 static void xilinx_dma_free_tx_segment(struct xilinx_dma_chan *chan,
struct xilinx_axidma_tx_segment *segment)
 {
-   dma_pool_free(chan->desc_pool, segment, segment->phys);
+   xilinx_dma_clean_hw_desc(>hw);
+
+   list_add_tail(>node, >free_seg_list);
 }
 
 /**
@@ -713,16 +736,26 @@ static void xilinx_dma_free_descriptors(struct 
xilinx_dma_chan *chan)
 static void xilinx_dma_free_chan_resources(struct dma_chan *dchan)
 {
struct xilinx_dma_chan *chan = to_xilinx_chan(dchan);
+   unsigned long flags;
 
dev_dbg(chan->dev, "Free all channel resources.\n");
 
xilinx_dma_free_descriptors(chan);
+
if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
-   xilinx_dma_free_tx_segment(chan, chan->cyclic_seg_v);
-   xilinx_dma_free_tx_segment(chan, chan->seg_v);
+   spin_lock_irqsave(>lock, flags);
+   INIT_LIST_HEAD(>free_seg_list);
+   spin_unlock_irqrestore(>lock, flags);
+
+   /* Free Memory that is allocated for cyclic DMA Mode */
+   dma_free_coherent(chan->dev, sizeof(*chan->cyclic_seg_v),
+ chan->cyclic_seg_v, 

[PATCH v3 3/3] dmaengine: xilinx_dma: Fix race condition in the driver for multiple descriptor scenario

2017-01-03 Thread Kedareswara rao Appana
When driver is handling AXI DMA SoftIP
When user submits multiple descriptors back to back on the S2MM(recv)
side with the current driver flow the last buffer descriptor next bd
points to a invalid location resulting the invalid data or errors in the
DMA engine.

This patch fixes this issue by creating a BD Chain during
channel allocation itself and use those BD's.

Signed-off-by: Kedareswara rao Appana 
---
Changes for v3:
---> None.
Changes for v2:
---> None.

 drivers/dma/xilinx/xilinx_dma.c | 133 +---
 1 file changed, 83 insertions(+), 50 deletions(-)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 7cd8e08..822ccf00 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -163,6 +163,7 @@
 #define XILINX_DMA_BD_SOP  BIT(27)
 #define XILINX_DMA_BD_EOP  BIT(26)
 #define XILINX_DMA_COALESCE_MAX255
+#define XILINX_DMA_NUM_DESCS   255
 #define XILINX_DMA_NUM_APP_WORDS   5
 
 /* Multi-Channel DMA Descriptor offsets*/
@@ -310,6 +311,7 @@ struct xilinx_dma_tx_descriptor {
  * @pending_list: Descriptors waiting
  * @active_list: Descriptors ready to submit
  * @done_list: Complete descriptors
+ * @free_seg_list: Free descriptors
  * @common: DMA common channel
  * @desc_pool: Descriptors pool
  * @dev: The dma device
@@ -331,7 +333,9 @@ struct xilinx_dma_tx_descriptor {
  * @desc_submitcount: Descriptor h/w submitted count
  * @residue: Residue for AXI DMA
  * @seg_v: Statically allocated segments base
+ * @seg_p: Physical allocated segments base
  * @cyclic_seg_v: Statically allocated segment base for cyclic transfers
+ * @cyclic_seg_p: Physical allocated segments base for cyclic dma
  * @start_transfer: Differentiate b/w DMA IP's transfer
  */
 struct xilinx_dma_chan {
@@ -342,6 +346,7 @@ struct xilinx_dma_chan {
struct list_head pending_list;
struct list_head active_list;
struct list_head done_list;
+   struct list_head free_seg_list;
struct dma_chan common;
struct dma_pool *desc_pool;
struct device *dev;
@@ -363,7 +368,9 @@ struct xilinx_dma_chan {
u32 desc_submitcount;
u32 residue;
struct xilinx_axidma_tx_segment *seg_v;
+   dma_addr_t seg_p;
struct xilinx_axidma_tx_segment *cyclic_seg_v;
+   dma_addr_t cyclic_seg_p;
void (*start_transfer)(struct xilinx_dma_chan *chan);
u16 tdest;
 };
@@ -569,17 +576,31 @@ static struct xilinx_axidma_tx_segment *
 xilinx_axidma_alloc_tx_segment(struct xilinx_dma_chan *chan)
 {
struct xilinx_axidma_tx_segment *segment;
-   dma_addr_t phys;
-
-   segment = dma_pool_zalloc(chan->desc_pool, GFP_ATOMIC, );
-   if (!segment)
-   return NULL;
+   unsigned long flags;
 
-   segment->phys = phys;
+   spin_lock_irqsave(>lock, flags);
+   if (!list_empty(>free_seg_list)) {
+   segment = list_first_entry(>free_seg_list,
+  struct xilinx_axidma_tx_segment,
+  node);
+   list_del(>node);
+   }
+   spin_unlock_irqrestore(>lock, flags);
 
return segment;
 }
 
+static void xilinx_dma_clean_hw_desc(struct xilinx_axidma_desc_hw *hw)
+{
+   u32 next_desc = hw->next_desc;
+   u32 next_desc_msb = hw->next_desc_msb;
+
+   memset(hw, 0, sizeof(struct xilinx_axidma_desc_hw));
+
+   hw->next_desc = next_desc;
+   hw->next_desc_msb = next_desc_msb;
+}
+
 /**
  * xilinx_dma_free_tx_segment - Free transaction segment
  * @chan: Driver specific DMA channel
@@ -588,7 +609,9 @@ xilinx_axidma_alloc_tx_segment(struct xilinx_dma_chan *chan)
 static void xilinx_dma_free_tx_segment(struct xilinx_dma_chan *chan,
struct xilinx_axidma_tx_segment *segment)
 {
-   dma_pool_free(chan->desc_pool, segment, segment->phys);
+   xilinx_dma_clean_hw_desc(>hw);
+
+   list_add_tail(>node, >free_seg_list);
 }
 
 /**
@@ -713,16 +736,26 @@ static void xilinx_dma_free_descriptors(struct 
xilinx_dma_chan *chan)
 static void xilinx_dma_free_chan_resources(struct dma_chan *dchan)
 {
struct xilinx_dma_chan *chan = to_xilinx_chan(dchan);
+   unsigned long flags;
 
dev_dbg(chan->dev, "Free all channel resources.\n");
 
xilinx_dma_free_descriptors(chan);
+
if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
-   xilinx_dma_free_tx_segment(chan, chan->cyclic_seg_v);
-   xilinx_dma_free_tx_segment(chan, chan->seg_v);
+   spin_lock_irqsave(>lock, flags);
+   INIT_LIST_HEAD(>free_seg_list);
+   spin_unlock_irqrestore(>lock, flags);
+
+   /* Free Memory that is allocated for cyclic DMA Mode */
+   dma_free_coherent(chan->dev, sizeof(*chan->cyclic_seg_v),
+ chan->cyclic_seg_v, chan->cyclic_seg_p);

[PATCH v3 0/3] dmaengine: xilinx_dma: Bug fixes

2017-01-03 Thread Kedareswara rao Appana
This patch series fixes below bugs in DMA and VDMA IP's
---> Do not start VDMA until frame buffer is processed by the h/w Fix 
---> bug in Multi frame sotres handling in VDMA Fix issues w.r.to multi 
---> frame descriptors submit with AXI DMA S2MM(recv) Side.

Kedareswara rao Appana (3):
  dmaengine: xilinx_dma: Check for channel idle state before submitting
dma descriptor
  dmaeninge: xilinx_dma: Fix bug in multiple frame stores scenario in
vdma
  dmaengine: xilinx_dma: Fix race condition in the driver for multiple
descriptor scenario

 .../devicetree/bindings/dma/xilinx/xilinx_dma.txt  |   2 +
 drivers/dma/xilinx/xilinx_dma.c| 265 -
 2 files changed, 156 insertions(+), 111 deletions(-)

-- 
2.1.2



[PATCH v3 0/3] dmaengine: xilinx_dma: Bug fixes

2017-01-03 Thread Kedareswara rao Appana
This patch series fixes below bugs in DMA and VDMA IP's
---> Do not start VDMA until frame buffer is processed by the h/w Fix 
---> bug in Multi frame sotres handling in VDMA Fix issues w.r.to multi 
---> frame descriptors submit with AXI DMA S2MM(recv) Side.

Kedareswara rao Appana (3):
  dmaengine: xilinx_dma: Check for channel idle state before submitting
dma descriptor
  dmaeninge: xilinx_dma: Fix bug in multiple frame stores scenario in
vdma
  dmaengine: xilinx_dma: Fix race condition in the driver for multiple
descriptor scenario

 .../devicetree/bindings/dma/xilinx/xilinx_dma.txt  |   2 +
 drivers/dma/xilinx/xilinx_dma.c| 265 -
 2 files changed, 156 insertions(+), 111 deletions(-)

-- 
2.1.2



[PATCH v3 2/3] dmaeninge: xilinx_dma: Fix bug in multiple frame stores scenario in vdma

2017-01-03 Thread Kedareswara rao Appana
When VDMA is configured for more than one frame in the h/w
for example h/w is configured for n number of frames and user
Submits n number of frames and triggered the DMA using issue_pending API.
In the current driver flow we are submitting one frame at a time
but we should submit all the n number of frames at one time as the h/w
Is configured for n number of frames.

This patch fixes this issue.

Signed-off-by: Kedareswara rao Appana 
---
Changes for v3:
---> Added Checks for frame store configuration. If frame store
 Configuration is not present at the h/w level and user
 Submits less frames added debug prints in the driver as relevant.
Changes for v2:
---> Fixed race conditions in the driver as suggested by Jose Abreu
---> Fixed unnecessray if else checks in the vdma_start_transfer
 as suggested by Laurent Pinchart.

 .../devicetree/bindings/dma/xilinx/xilinx_dma.txt  |  2 +
 drivers/dma/xilinx/xilinx_dma.c| 78 +++---
 2 files changed, 57 insertions(+), 23 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt 
b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
index a2b8bfa..1f65e09 100644
--- a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
+++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
@@ -66,6 +66,8 @@ Optional child node properties:
 Optional child node properties for VDMA:
 - xlnx,genlock-mode: Tells Genlock synchronization is
enabled/disabled in hardware.
+- xlnx,fstore-config: Tells Whether Frame Store Configuration is
+   enabled/disabled in hardware.
 Optional child node properties for AXI DMA:
 -dma-channels: Number of dma channels in child node.
 
diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index be7eb41..7cd8e08 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -322,6 +322,7 @@ struct xilinx_dma_tx_descriptor {
  * @genlock: Support genlock mode
  * @err: Channel has errors
  * @idle: Check for channel idle
+ * @has_fstoreconfig: Check for frame store configuration
  * @tasklet: Cleanup work after irq
  * @config: Device configuration info
  * @flush_on_fsync: Flush on Frame sync
@@ -353,6 +354,7 @@ struct xilinx_dma_chan {
bool genlock;
bool err;
bool idle;
+   bool has_fstoreconfig;
struct tasklet_struct tasklet;
struct xilinx_vdma_config config;
bool flush_on_fsync;
@@ -990,6 +992,26 @@ static void xilinx_vdma_start_transfer(struct 
xilinx_dma_chan *chan)
if (list_empty(>pending_list))
return;
 
+   /*
+* Note: When VDMA is built with default h/w configuration
+* On the S2MM(recv) side user should submit frames upto
+* H/W configured. If users submits less than h/w configured
+* VDMA engine tries to write to a invalid location
+* Results undefined behaviour/memory corruption.
+*
+* If user would like to submit frames less than h/w capable
+* On S2MM side please enable debug info 13 at the h/w level
+* It will allows the frame buffers numbers to be modified at runtime.
+*/
+   if (!chan->has_fstoreconfig && chan->direction == DMA_DEV_TO_MEM &&
+   chan->desc_pendingcount < chan->num_frms) {
+   dev_dbg(chan->dev, "Frame Store Configuration is not enabled at 
the");
+   dev_dbg(chan->dev, " H/w level enable Debug info 13 at the h/w 
level");
+   dev_dbg(chan->dev, " OR Submit the frames upto h/w 
Capable\n\r");
+
+   return;
+   }
+
desc = list_first_entry(>pending_list,
struct xilinx_dma_tx_descriptor, node);
tail_desc = list_last_entry(>pending_list,
@@ -1052,25 +1074,38 @@ static void xilinx_vdma_start_transfer(struct 
xilinx_dma_chan *chan)
if (chan->has_sg) {
dma_ctrl_write(chan, XILINX_DMA_REG_TAILDESC,
tail_segment->phys);
+   list_splice_tail_init(>pending_list, >active_list);
+   chan->desc_pendingcount = 0;
} else {
struct xilinx_vdma_tx_segment *segment, *last = NULL;
-   int i = 0;
+   int i = 0, j = 0;
 
if (chan->desc_submitcount < chan->num_frms)
i = chan->desc_submitcount;
 
-   list_for_each_entry(segment, >segments, node) {
-   if (chan->ext_addr)
-   vdma_desc_write_64(chan,
-   XILINX_VDMA_REG_START_ADDRESS_64(i++),
-   segment->hw.buf_addr,
-   segment->hw.buf_addr_msb);
-   else
-   vdma_desc_write(chan,
-   XILINX_VDMA_REG_START_ADDRESS(i++),
-

[PATCH v3 2/3] dmaeninge: xilinx_dma: Fix bug in multiple frame stores scenario in vdma

2017-01-03 Thread Kedareswara rao Appana
When VDMA is configured for more than one frame in the h/w
for example h/w is configured for n number of frames and user
Submits n number of frames and triggered the DMA using issue_pending API.
In the current driver flow we are submitting one frame at a time
but we should submit all the n number of frames at one time as the h/w
Is configured for n number of frames.

This patch fixes this issue.

Signed-off-by: Kedareswara rao Appana 
---
Changes for v3:
---> Added Checks for frame store configuration. If frame store
 Configuration is not present at the h/w level and user
 Submits less frames added debug prints in the driver as relevant.
Changes for v2:
---> Fixed race conditions in the driver as suggested by Jose Abreu
---> Fixed unnecessray if else checks in the vdma_start_transfer
 as suggested by Laurent Pinchart.

 .../devicetree/bindings/dma/xilinx/xilinx_dma.txt  |  2 +
 drivers/dma/xilinx/xilinx_dma.c| 78 +++---
 2 files changed, 57 insertions(+), 23 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt 
b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
index a2b8bfa..1f65e09 100644
--- a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
+++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
@@ -66,6 +66,8 @@ Optional child node properties:
 Optional child node properties for VDMA:
 - xlnx,genlock-mode: Tells Genlock synchronization is
enabled/disabled in hardware.
+- xlnx,fstore-config: Tells Whether Frame Store Configuration is
+   enabled/disabled in hardware.
 Optional child node properties for AXI DMA:
 -dma-channels: Number of dma channels in child node.
 
diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index be7eb41..7cd8e08 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -322,6 +322,7 @@ struct xilinx_dma_tx_descriptor {
  * @genlock: Support genlock mode
  * @err: Channel has errors
  * @idle: Check for channel idle
+ * @has_fstoreconfig: Check for frame store configuration
  * @tasklet: Cleanup work after irq
  * @config: Device configuration info
  * @flush_on_fsync: Flush on Frame sync
@@ -353,6 +354,7 @@ struct xilinx_dma_chan {
bool genlock;
bool err;
bool idle;
+   bool has_fstoreconfig;
struct tasklet_struct tasklet;
struct xilinx_vdma_config config;
bool flush_on_fsync;
@@ -990,6 +992,26 @@ static void xilinx_vdma_start_transfer(struct 
xilinx_dma_chan *chan)
if (list_empty(>pending_list))
return;
 
+   /*
+* Note: When VDMA is built with default h/w configuration
+* On the S2MM(recv) side user should submit frames upto
+* H/W configured. If users submits less than h/w configured
+* VDMA engine tries to write to a invalid location
+* Results undefined behaviour/memory corruption.
+*
+* If user would like to submit frames less than h/w capable
+* On S2MM side please enable debug info 13 at the h/w level
+* It will allows the frame buffers numbers to be modified at runtime.
+*/
+   if (!chan->has_fstoreconfig && chan->direction == DMA_DEV_TO_MEM &&
+   chan->desc_pendingcount < chan->num_frms) {
+   dev_dbg(chan->dev, "Frame Store Configuration is not enabled at 
the");
+   dev_dbg(chan->dev, " H/w level enable Debug info 13 at the h/w 
level");
+   dev_dbg(chan->dev, " OR Submit the frames upto h/w 
Capable\n\r");
+
+   return;
+   }
+
desc = list_first_entry(>pending_list,
struct xilinx_dma_tx_descriptor, node);
tail_desc = list_last_entry(>pending_list,
@@ -1052,25 +1074,38 @@ static void xilinx_vdma_start_transfer(struct 
xilinx_dma_chan *chan)
if (chan->has_sg) {
dma_ctrl_write(chan, XILINX_DMA_REG_TAILDESC,
tail_segment->phys);
+   list_splice_tail_init(>pending_list, >active_list);
+   chan->desc_pendingcount = 0;
} else {
struct xilinx_vdma_tx_segment *segment, *last = NULL;
-   int i = 0;
+   int i = 0, j = 0;
 
if (chan->desc_submitcount < chan->num_frms)
i = chan->desc_submitcount;
 
-   list_for_each_entry(segment, >segments, node) {
-   if (chan->ext_addr)
-   vdma_desc_write_64(chan,
-   XILINX_VDMA_REG_START_ADDRESS_64(i++),
-   segment->hw.buf_addr,
-   segment->hw.buf_addr_msb);
-   else
-   vdma_desc_write(chan,
-   XILINX_VDMA_REG_START_ADDRESS(i++),
-   

Re: [PATCH v3] IB/umem: Release pid in error and ODP flow

2017-01-03 Thread Leon Romanovsky
On Wed, Jan 04, 2017 at 03:07:04PM +0800, Kenneth Lee wrote:
> On Tue, Jan 03, 2017 at 12:12:24PM +0200, Leon Romanovsky wrote:
> > Date: Tue, 3 Jan 2017 12:12:24 +0200
> > From: Leon Romanovsky 
> > To: Kenneth Lee 
> > CC: dledf...@redhat.com, sean.he...@intel.com, hal.rosenst...@gmail.com,
> >  robin.mur...@arm.com, jroe...@suse.de, egtv...@samfundet.no,
> >  vgu...@synopsys.com, dave.han...@linux.intel.com, lstoa...@gmail.com,
> >  k...@kernel.org, seb...@linux.vnet.ibm.com, ma...@mellanox.com,
> >  linux-r...@vger.kernel.org, linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH v3] IB/umem: Release pid in error and ODP flow
> > User-Agent: Mutt/1.7.2 (2016-11-26)
> > Message-ID: <20170103101224.GH12077@mtr-leonro.local>
> >
> > On Tue, Jan 03, 2017 at 10:32:50AM +0800, Kenneth Lee wrote:
> > > On Sun, Jan 01, 2017 at 08:47:12AM +0200, Leon Romanovsky wrote:
> > > > Date: Sun, 1 Jan 2017 08:47:12 +0200
> > > > From: Leon Romanovsky 
> > > > To: Kenneth Lee 
> > > > CC: dledf...@redhat.com, sean.he...@intel.com, hal.rosenst...@gmail.com,
> > > >  robin.mur...@arm.com, jroe...@suse.de, egtv...@samfundet.no,
> > > >  vgu...@synopsys.com, dave.han...@linux.intel.com, lstoa...@gmail.com,
> > > >  k...@kernel.org, seb...@linux.vnet.ibm.com, ma...@mellanox.com,
> > > >  linux-r...@vger.kernel.org, linux-kernel@vger.kernel.org
> > > > Subject: Re: [PATCH v3] IB/umem: Release pid in error and ODP flow
> > > > User-Agent: Mutt/1.7.2 (2016-11-26)
> > > > Message-ID: <20170101064712.GQ26885@mtr-leonro.local>
> > > >
> > > > On Fri, Dec 30, 2016 at 06:18:29PM +0800, Kenneth Lee wrote:
> > > > > There are two bugfixes in this patch:
> > > > >
> > > > > Fixes: 87773dd56d54 ("IB: ib_umem_release() should decrement 
> > > > > mm->pinned_vm from ib_umem_get")
> > > > >   This patch introduce the get_task_pid but not put it back on 
> > > > > all error
> > > > >   path
> > > > >
> > > > > Fixes: 8ada2c1c0c1d ("IB/core: Add support for on demand paging 
> > > > > regions")
> > > > >   This patch introduce a ODP flow without release pid before 
> > > > > enter it
> > > > >
> > > > >
> > > > > Signed-off-by: Kenneth Lee 
> > > > > Reviewed-by: Haggai Eran 
> > > > > ---
> > > > > Change from v1 to v2:
> > > > >   Correcting the patch title and description
> > > > > Change from v2 to v3:
> > > > >   Update the title and add "Fixes" fields in the description
> > > >
> > > > OK,
> > > >
> > > > I see that you still didn't read Documentation/SubmittingPatches. You
> > > > must read that document before you are sending patches.
> > > >
> > > > But I'll stop here, the code is correct (it fixes bugs) and commit 
> > > > message
> > > > more usefull than before.
> > > >
> > > >
> > > > >
> > > > >  drivers/infiniband/core/umem.c | 2 ++
> > > > >  1 file changed, 2 insertions(+)
> > > > >
> > > > > diff --git a/drivers/infiniband/core/umem.c 
> > > > > b/drivers/infiniband/core/umem.c
> > > > > index 1e62a5f..4609b92 100644
> > > > > --- a/drivers/infiniband/core/umem.c
> > > > > +++ b/drivers/infiniband/core/umem.c
> > > > > @@ -134,6 +134,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext 
> > > > > *context, unsigned long addr,
> > > > >IB_ACCESS_REMOTE_ATOMIC | IB_ACCESS_MW_BIND));
> > > > >
> > > > >   if (access & IB_ACCESS_ON_DEMAND) {
> > > > > + put_pid(umem->pid);
> > > > >   ret = ib_umem_odp_get(context, umem);
> > > > >   if (ret) {
> > > > >   kfree(umem);
> > > > > @@ -149,6 +150,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext 
> > > > > *context, unsigned long addr,
> > > > >
> > > > >   page_list = (struct page **) __get_free_page(GFP_KERNEL);
> > > > >   if (!page_list) {
> > > > > + put_pid(umem->pid);
> > > > >   kfree(umem);
> > > > >   return ERR_PTR(-ENOMEM);
> > > > >   }
> > > > > --
> > > > > 1.9.1
> > > > >
> > > > > --
> > > > > To unsubscribe from this list: send the line "unsubscribe linux-rdma" 
> > > > > in
> > > > > the body of a message to majord...@vger.kernel.org
> > > > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > >
> > > Thanks,
> > >
> > > I did read the doc, but maybe I mis-understant some points. Could you 
> > > please
> > > point it out?
> >
> > Fixes line should be placed above bottom signatures.
> >
> > As an example of properly written patch, you can take a look on the
> > following patch [1] from Steve.
> >
> > [1] http://marc.info/?l=linux-rdma=148244272205411=2
>
> Thank you. A sample help a lot.
>
> But please allow me to argue a little:
> Documentation/process/submitting-patches.rst does really not mention where 
> Fixes
> tags should be put:)

Sure, you can't and don't want to document everything. There is section named
"2) Describe your changes" which has very extensive description about

Re: [PATCH v3] IB/umem: Release pid in error and ODP flow

2017-01-03 Thread Leon Romanovsky
On Wed, Jan 04, 2017 at 03:07:04PM +0800, Kenneth Lee wrote:
> On Tue, Jan 03, 2017 at 12:12:24PM +0200, Leon Romanovsky wrote:
> > Date: Tue, 3 Jan 2017 12:12:24 +0200
> > From: Leon Romanovsky 
> > To: Kenneth Lee 
> > CC: dledf...@redhat.com, sean.he...@intel.com, hal.rosenst...@gmail.com,
> >  robin.mur...@arm.com, jroe...@suse.de, egtv...@samfundet.no,
> >  vgu...@synopsys.com, dave.han...@linux.intel.com, lstoa...@gmail.com,
> >  k...@kernel.org, seb...@linux.vnet.ibm.com, ma...@mellanox.com,
> >  linux-r...@vger.kernel.org, linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH v3] IB/umem: Release pid in error and ODP flow
> > User-Agent: Mutt/1.7.2 (2016-11-26)
> > Message-ID: <20170103101224.GH12077@mtr-leonro.local>
> >
> > On Tue, Jan 03, 2017 at 10:32:50AM +0800, Kenneth Lee wrote:
> > > On Sun, Jan 01, 2017 at 08:47:12AM +0200, Leon Romanovsky wrote:
> > > > Date: Sun, 1 Jan 2017 08:47:12 +0200
> > > > From: Leon Romanovsky 
> > > > To: Kenneth Lee 
> > > > CC: dledf...@redhat.com, sean.he...@intel.com, hal.rosenst...@gmail.com,
> > > >  robin.mur...@arm.com, jroe...@suse.de, egtv...@samfundet.no,
> > > >  vgu...@synopsys.com, dave.han...@linux.intel.com, lstoa...@gmail.com,
> > > >  k...@kernel.org, seb...@linux.vnet.ibm.com, ma...@mellanox.com,
> > > >  linux-r...@vger.kernel.org, linux-kernel@vger.kernel.org
> > > > Subject: Re: [PATCH v3] IB/umem: Release pid in error and ODP flow
> > > > User-Agent: Mutt/1.7.2 (2016-11-26)
> > > > Message-ID: <20170101064712.GQ26885@mtr-leonro.local>
> > > >
> > > > On Fri, Dec 30, 2016 at 06:18:29PM +0800, Kenneth Lee wrote:
> > > > > There are two bugfixes in this patch:
> > > > >
> > > > > Fixes: 87773dd56d54 ("IB: ib_umem_release() should decrement 
> > > > > mm->pinned_vm from ib_umem_get")
> > > > >   This patch introduce the get_task_pid but not put it back on 
> > > > > all error
> > > > >   path
> > > > >
> > > > > Fixes: 8ada2c1c0c1d ("IB/core: Add support for on demand paging 
> > > > > regions")
> > > > >   This patch introduce a ODP flow without release pid before 
> > > > > enter it
> > > > >
> > > > >
> > > > > Signed-off-by: Kenneth Lee 
> > > > > Reviewed-by: Haggai Eran 
> > > > > ---
> > > > > Change from v1 to v2:
> > > > >   Correcting the patch title and description
> > > > > Change from v2 to v3:
> > > > >   Update the title and add "Fixes" fields in the description
> > > >
> > > > OK,
> > > >
> > > > I see that you still didn't read Documentation/SubmittingPatches. You
> > > > must read that document before you are sending patches.
> > > >
> > > > But I'll stop here, the code is correct (it fixes bugs) and commit 
> > > > message
> > > > more usefull than before.
> > > >
> > > >
> > > > >
> > > > >  drivers/infiniband/core/umem.c | 2 ++
> > > > >  1 file changed, 2 insertions(+)
> > > > >
> > > > > diff --git a/drivers/infiniband/core/umem.c 
> > > > > b/drivers/infiniband/core/umem.c
> > > > > index 1e62a5f..4609b92 100644
> > > > > --- a/drivers/infiniband/core/umem.c
> > > > > +++ b/drivers/infiniband/core/umem.c
> > > > > @@ -134,6 +134,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext 
> > > > > *context, unsigned long addr,
> > > > >IB_ACCESS_REMOTE_ATOMIC | IB_ACCESS_MW_BIND));
> > > > >
> > > > >   if (access & IB_ACCESS_ON_DEMAND) {
> > > > > + put_pid(umem->pid);
> > > > >   ret = ib_umem_odp_get(context, umem);
> > > > >   if (ret) {
> > > > >   kfree(umem);
> > > > > @@ -149,6 +150,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext 
> > > > > *context, unsigned long addr,
> > > > >
> > > > >   page_list = (struct page **) __get_free_page(GFP_KERNEL);
> > > > >   if (!page_list) {
> > > > > + put_pid(umem->pid);
> > > > >   kfree(umem);
> > > > >   return ERR_PTR(-ENOMEM);
> > > > >   }
> > > > > --
> > > > > 1.9.1
> > > > >
> > > > > --
> > > > > To unsubscribe from this list: send the line "unsubscribe linux-rdma" 
> > > > > in
> > > > > the body of a message to majord...@vger.kernel.org
> > > > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > >
> > > Thanks,
> > >
> > > I did read the doc, but maybe I mis-understant some points. Could you 
> > > please
> > > point it out?
> >
> > Fixes line should be placed above bottom signatures.
> >
> > As an example of properly written patch, you can take a look on the
> > following patch [1] from Steve.
> >
> > [1] http://marc.info/?l=linux-rdma=148244272205411=2
>
> Thank you. A sample help a lot.
>
> But please allow me to argue a little:
> Documentation/process/submitting-patches.rst does really not mention where 
> Fixes
> tags should be put:)

Sure, you can't and don't want to document everything. There is section named
"2) Describe your changes" which has very extensive description about
commit message. If you had followed it, you would find that it is very
natural to place Fixes at the bottom.

Thanks

>
> >
> > 

Re: [PATCH 1/2] pwm: sunxi: allow the pwm to finish its pulse before disable

2017-01-03 Thread Thierry Reding
On Tue, Jan 03, 2017 at 05:55:54PM +0100, Alexandre Belloni wrote:
> On 03/01/2017 at 16:59:57 +0100, Olliver Schinagl wrote :
> > On 12-12-16 13:24, Maxime Ripard wrote:
> > > On Thu, Dec 08, 2016 at 02:23:39PM +0100, Olliver Schinagl wrote:
> > > > Hey Maxime,
> > > > 
> > > > first off, also sorry for the slow delay :) (pun not intended)
> > > > 
> > > > On 27-08-16 00:19, Maxime Ripard wrote:
> > > > > On Thu, Aug 25, 2016 at 07:50:10PM +0200, Olliver Schinagl wrote:
> > > > > > When we inform the PWM block to stop toggeling the output, we may 
> > > > > > end up
> > > > > > in a state where the output is not what we would expect (e.g. not 
> > > > > > the
> > > > > > low-pulse) but whatever the output was at when the clock got 
> > > > > > disabled.
> > > > > > 
> > > > > > To counter this we have to wait for maximally the time of one whole
> > > > > > period to ensure the pwm hardware was able to finish. Since we 
> > > > > > already
> > > > > > told the PWM hardware to disable it self, it will not continue 
> > > > > > toggling
> > > > > > but merly finish its current pulse.
> > > > > > 
> > > > > > If a whole period is considered to much, it may be contemplated to 
> > > > > > use a
> > > > > > half period + a little bit to ensure we get passed the transition.
> > > > > > 
> > > > > > Signed-off-by: Olliver Schinagl
> > > > > > ---
> > > > > >   drivers/pwm/pwm-sun4i.c | 11 +++
> > > > > >   1 file changed, 11 insertions(+)
> > > > > > 
> > > > > > diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> > > > > > index 03a99a5..5e97c8a 100644
> > > > > > --- a/drivers/pwm/pwm-sun4i.c
> > > > > > +++ b/drivers/pwm/pwm-sun4i.c
> > > > > > @@ -8,6 +8,7 @@
> > > > > >   #include 
> > > > > >   #include 
> > > > > > +#include 
> > > > > >   #include 
> > > > > >   #include 
> > > > > >   #include 
> > > > > > @@ -245,6 +246,16 @@ static void sun4i_pwm_disable(struct pwm_chip 
> > > > > > *chip, struct pwm_device *pwm)
> > > > > > spin_lock(_pwm->ctrl_lock);
> > > > > > val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
> > > > > > val &= ~BIT_CH(PWM_EN, pwm->hwpwm);
> > > > > > +   sun4i_pwm_writel(sun4i_pwm, val, PWM_CTRL_REG);
> > > > > > +   spin_unlock(_pwm->ctrl_lock);
> > > > > > +
> > > > > > +   /* Allow for the PWM hardware to finish its last toggle. The 
> > > > > > pulse
> > > > > > +* may have just started and thus we should wait a full period.
> > > > > > +*/
> > > > > > +   ndelay(pwm_get_period(pwm));
> > > > > Can't that use the ready bit as well?
> > > > 
> > > > I started to implement our earlier discussed suggestions, but I do not 
> > > > think
> > > > they will work. The read bit is not to let the user know it is ready 
> > > > with
> > > > all of its commands, but only if the period registers are ready. I 
> > > > think it
> > > > is some write lock while it copies the data into its internal control 
> > > > loop.
> > > > From the manual:
> > > > PWM0 period register ready.
> > > > 0: PWM0 period register is ready to write,
> > > > 1: PWM0 period register is busy.
> > > > 
> > > > 
> > > > So no, I don't think i can use the ready bit here at all. The only 
> > > > thing we
> > > > can do here, but I doubt it's worth it, is to read the period register,
> > > > caluclate a time from it, and then ndelay(pwm_get_period(pwm) - 
> > > > ran_time)
> > > > 
> > > > The only 'win' then is that we could are potentially not waiting the 
> > > > full
> > > > pwm period, but only a fraction of it. Since we are disabling the 
> > > > hardware
> > > > (for power reasons) anyway, I don't think this is any significant win,
> > > > except for extreme situations. E.g. we have a pwm period of 10 seconds, 
> > > > we
> > > > disable it after 9.9 second, and now we have to wait for 10 seconds 
> > > > before
> > > > the pwm_disable is finally done. So this could in that case be reduced 
> > > > to
> > > > then only wait for 0.2 seconds since it is 'done' sooner.
> > > > 
> > > > However that optimization is also not 'free'. We have to read the period
> > > > register and calculate back the time. I suggest to do that when 
> > > > reworking
> > > > this driver to work with atomic mode, and merge this patch 'as is' to
> > > > atleast fix te bug where simply not finish properly.
> > > 
> > > That whole discussion made me realise something that is really
> > > bad. AFAIK, pwm_get_period returns a 32 bits register, which means a
> > > theorical period of 4s. Busy looping during 4 seconds is already very
> > > bad, as you basically kill one CPU during that time, but doing so in a
> > > (potentially) atomic context is even worse.
> > Well technically, isn't it a 16 bit register? (half for the period, other
> > half for the duty cycle?) Anyway, I think the delay can be far exceeding 4
> > seconds (though I haven't checked what the PWM delay max option is).
> > 
> 
> That's 196.8s.
> 
> But you can rely on the RDY bit because what you want to
> achieve 

Re: [PATCH 1/2] pwm: sunxi: allow the pwm to finish its pulse before disable

2017-01-03 Thread Thierry Reding
On Tue, Jan 03, 2017 at 05:55:54PM +0100, Alexandre Belloni wrote:
> On 03/01/2017 at 16:59:57 +0100, Olliver Schinagl wrote :
> > On 12-12-16 13:24, Maxime Ripard wrote:
> > > On Thu, Dec 08, 2016 at 02:23:39PM +0100, Olliver Schinagl wrote:
> > > > Hey Maxime,
> > > > 
> > > > first off, also sorry for the slow delay :) (pun not intended)
> > > > 
> > > > On 27-08-16 00:19, Maxime Ripard wrote:
> > > > > On Thu, Aug 25, 2016 at 07:50:10PM +0200, Olliver Schinagl wrote:
> > > > > > When we inform the PWM block to stop toggeling the output, we may 
> > > > > > end up
> > > > > > in a state where the output is not what we would expect (e.g. not 
> > > > > > the
> > > > > > low-pulse) but whatever the output was at when the clock got 
> > > > > > disabled.
> > > > > > 
> > > > > > To counter this we have to wait for maximally the time of one whole
> > > > > > period to ensure the pwm hardware was able to finish. Since we 
> > > > > > already
> > > > > > told the PWM hardware to disable it self, it will not continue 
> > > > > > toggling
> > > > > > but merly finish its current pulse.
> > > > > > 
> > > > > > If a whole period is considered to much, it may be contemplated to 
> > > > > > use a
> > > > > > half period + a little bit to ensure we get passed the transition.
> > > > > > 
> > > > > > Signed-off-by: Olliver Schinagl
> > > > > > ---
> > > > > >   drivers/pwm/pwm-sun4i.c | 11 +++
> > > > > >   1 file changed, 11 insertions(+)
> > > > > > 
> > > > > > diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> > > > > > index 03a99a5..5e97c8a 100644
> > > > > > --- a/drivers/pwm/pwm-sun4i.c
> > > > > > +++ b/drivers/pwm/pwm-sun4i.c
> > > > > > @@ -8,6 +8,7 @@
> > > > > >   #include 
> > > > > >   #include 
> > > > > > +#include 
> > > > > >   #include 
> > > > > >   #include 
> > > > > >   #include 
> > > > > > @@ -245,6 +246,16 @@ static void sun4i_pwm_disable(struct pwm_chip 
> > > > > > *chip, struct pwm_device *pwm)
> > > > > > spin_lock(_pwm->ctrl_lock);
> > > > > > val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
> > > > > > val &= ~BIT_CH(PWM_EN, pwm->hwpwm);
> > > > > > +   sun4i_pwm_writel(sun4i_pwm, val, PWM_CTRL_REG);
> > > > > > +   spin_unlock(_pwm->ctrl_lock);
> > > > > > +
> > > > > > +   /* Allow for the PWM hardware to finish its last toggle. The 
> > > > > > pulse
> > > > > > +* may have just started and thus we should wait a full period.
> > > > > > +*/
> > > > > > +   ndelay(pwm_get_period(pwm));
> > > > > Can't that use the ready bit as well?
> > > > 
> > > > I started to implement our earlier discussed suggestions, but I do not 
> > > > think
> > > > they will work. The read bit is not to let the user know it is ready 
> > > > with
> > > > all of its commands, but only if the period registers are ready. I 
> > > > think it
> > > > is some write lock while it copies the data into its internal control 
> > > > loop.
> > > > From the manual:
> > > > PWM0 period register ready.
> > > > 0: PWM0 period register is ready to write,
> > > > 1: PWM0 period register is busy.
> > > > 
> > > > 
> > > > So no, I don't think i can use the ready bit here at all. The only 
> > > > thing we
> > > > can do here, but I doubt it's worth it, is to read the period register,
> > > > caluclate a time from it, and then ndelay(pwm_get_period(pwm) - 
> > > > ran_time)
> > > > 
> > > > The only 'win' then is that we could are potentially not waiting the 
> > > > full
> > > > pwm period, but only a fraction of it. Since we are disabling the 
> > > > hardware
> > > > (for power reasons) anyway, I don't think this is any significant win,
> > > > except for extreme situations. E.g. we have a pwm period of 10 seconds, 
> > > > we
> > > > disable it after 9.9 second, and now we have to wait for 10 seconds 
> > > > before
> > > > the pwm_disable is finally done. So this could in that case be reduced 
> > > > to
> > > > then only wait for 0.2 seconds since it is 'done' sooner.
> > > > 
> > > > However that optimization is also not 'free'. We have to read the period
> > > > register and calculate back the time. I suggest to do that when 
> > > > reworking
> > > > this driver to work with atomic mode, and merge this patch 'as is' to
> > > > atleast fix te bug where simply not finish properly.
> > > 
> > > That whole discussion made me realise something that is really
> > > bad. AFAIK, pwm_get_period returns a 32 bits register, which means a
> > > theorical period of 4s. Busy looping during 4 seconds is already very
> > > bad, as you basically kill one CPU during that time, but doing so in a
> > > (potentially) atomic context is even worse.
> > Well technically, isn't it a 16 bit register? (half for the period, other
> > half for the duty cycle?) Anyway, I think the delay can be far exceeding 4
> > seconds (though I haven't checked what the PWM delay max option is).
> > 
> 
> That's 196.8s.
> 
> But you can rely on the RDY bit because what you want to
> achieve actually relies on the 

RE: [PATCH 1/1] regulator: fixed: add suspend pm routines for pinctrl

2017-01-03 Thread Peter Chen
 
>>On Tue, Jan 03, 2017 at 05:02:47PM +0800, Peter Chen wrote:
>>> At some systems, the pinctrl setting will be lost or needs to set as
>>> "sleep" state to save power consumption. So, we need to configure
>>> pinctrl as "sleep" state when system enters suspend, and as "default"
>>> state after system resumes. In this way, the pinctrl value can be
>>> recovered as "default" state after resuming.
>>
>>I thought this was supposed to be done automatically by the driver core?
>>If not it should be, every single driver is likely to end up with that.
>
>Good idea, I will send a patch for that.
>

After checking more, it is not suitable for driver core do it since different 
driver
has different usage for "sleep" pinctrl. Below are some examples:

- Some drivers set "idle" for pinctrl when system goes to suspend
- Some drivers has some conditions for setting "sleep" for pinctrl
eg: drivers/net/ethernet/freescale/fec_main.c
- Some drivers may set "sleep" for pinctrl as ->close interface, but not system 
suspend
interface
- Some drivers can be woken up for "sleep" pinctrl, some may not.

Peter


RE: [PATCH 1/1] regulator: fixed: add suspend pm routines for pinctrl

2017-01-03 Thread Peter Chen
 
>>On Tue, Jan 03, 2017 at 05:02:47PM +0800, Peter Chen wrote:
>>> At some systems, the pinctrl setting will be lost or needs to set as
>>> "sleep" state to save power consumption. So, we need to configure
>>> pinctrl as "sleep" state when system enters suspend, and as "default"
>>> state after system resumes. In this way, the pinctrl value can be
>>> recovered as "default" state after resuming.
>>
>>I thought this was supposed to be done automatically by the driver core?
>>If not it should be, every single driver is likely to end up with that.
>
>Good idea, I will send a patch for that.
>

After checking more, it is not suitable for driver core do it since different 
driver
has different usage for "sleep" pinctrl. Below are some examples:

- Some drivers set "idle" for pinctrl when system goes to suspend
- Some drivers has some conditions for setting "sleep" for pinctrl
eg: drivers/net/ethernet/freescale/fec_main.c
- Some drivers may set "sleep" for pinctrl as ->close interface, but not system 
suspend
interface
- Some drivers can be woken up for "sleep" pinctrl, some may not.

Peter


[PATCH resend v4 2/3] ARM: dts: imx6: Support Savageboard dual

2017-01-03 Thread Milo Kim
Common savageboard DT file is used for board support.
Add the vendor name and specify the dtb file for i.MX6Q build.

Reviewed-by: Fabio Estevam 
Signed-off-by: Milo Kim 
---
 .../devicetree/bindings/vendor-prefixes.txt|  1 +
 arch/arm/boot/dts/Makefile |  1 +
 arch/arm/boot/dts/imx6dl-savageboard.dts   | 51 ++
 3 files changed, 53 insertions(+)
 create mode 100644 arch/arm/boot/dts/imx6dl-savageboard.dts

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt 
b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 16d3b5e7f5d1..552b63651ab9 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -227,6 +227,7 @@ pine64  Pine64
 pixcir  PIXCIR MICROELECTRONICS Co., Ltd
 plathome   Plat'Home Co., Ltd.
 plda   PLDA
+poslab Poslab Technology Co., Ltd.
 powervrPowerVR (deprecated, use img)
 pulsedlightPulsedLight, Inc
 qcaQualcomm Atheros, Inc.
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index cccdbcb557b6..fb7f904a5235 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -357,6 +357,7 @@ dtb-$(CONFIG_SOC_IMX6Q) += \
imx6dl-sabreauto.dtb \
imx6dl-sabrelite.dtb \
imx6dl-sabresd.dtb \
+   imx6dl-savageboard.dtb \
imx6dl-ts4900.dtb \
imx6dl-tx6dl-comtft.dtb \
imx6dl-tx6s-8034.dtb \
diff --git a/arch/arm/boot/dts/imx6dl-savageboard.dts 
b/arch/arm/boot/dts/imx6dl-savageboard.dts
new file mode 100644
index ..b95469c520a4
--- /dev/null
+++ b/arch/arm/boot/dts/imx6dl-savageboard.dts
@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) 2017 Milo Kim 
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This file is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+
+#include "imx6dl.dtsi"
+#include "imx6qdl-savageboard.dtsi"
+
+/ {
+   model = "Poslab SavageBoard Dual";
+   compatible = "poslab,imx6dl-savageboard", "fsl,imx6dl";
+};
-- 
2.11.0



[PATCH resend v4 2/3] ARM: dts: imx6: Support Savageboard dual

2017-01-03 Thread Milo Kim
Common savageboard DT file is used for board support.
Add the vendor name and specify the dtb file for i.MX6Q build.

Reviewed-by: Fabio Estevam 
Signed-off-by: Milo Kim 
---
 .../devicetree/bindings/vendor-prefixes.txt|  1 +
 arch/arm/boot/dts/Makefile |  1 +
 arch/arm/boot/dts/imx6dl-savageboard.dts   | 51 ++
 3 files changed, 53 insertions(+)
 create mode 100644 arch/arm/boot/dts/imx6dl-savageboard.dts

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt 
b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 16d3b5e7f5d1..552b63651ab9 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -227,6 +227,7 @@ pine64  Pine64
 pixcir  PIXCIR MICROELECTRONICS Co., Ltd
 plathome   Plat'Home Co., Ltd.
 plda   PLDA
+poslab Poslab Technology Co., Ltd.
 powervrPowerVR (deprecated, use img)
 pulsedlightPulsedLight, Inc
 qcaQualcomm Atheros, Inc.
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index cccdbcb557b6..fb7f904a5235 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -357,6 +357,7 @@ dtb-$(CONFIG_SOC_IMX6Q) += \
imx6dl-sabreauto.dtb \
imx6dl-sabrelite.dtb \
imx6dl-sabresd.dtb \
+   imx6dl-savageboard.dtb \
imx6dl-ts4900.dtb \
imx6dl-tx6dl-comtft.dtb \
imx6dl-tx6s-8034.dtb \
diff --git a/arch/arm/boot/dts/imx6dl-savageboard.dts 
b/arch/arm/boot/dts/imx6dl-savageboard.dts
new file mode 100644
index ..b95469c520a4
--- /dev/null
+++ b/arch/arm/boot/dts/imx6dl-savageboard.dts
@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) 2017 Milo Kim 
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This file is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+
+#include "imx6dl.dtsi"
+#include "imx6qdl-savageboard.dtsi"
+
+/ {
+   model = "Poslab SavageBoard Dual";
+   compatible = "poslab,imx6dl-savageboard", "fsl,imx6dl";
+};
-- 
2.11.0



Re: [PATCH] rcu: fix the OOM problem of huge IP abnormal packet traffic

2017-01-03 Thread Ding Tianhong


On 2017/1/4 8:57, Paul E. McKenney wrote:
> On Wed, Dec 28, 2016 at 04:13:15PM -0800, Paul E. McKenney wrote:
>> On Wed, Dec 28, 2016 at 01:58:06PM +0800, Ding Tianhong wrote:
>>> Hi, Paul:
>>>
>>> I try to debug this problem and found this solution could work well for 
>>> both problem scene.
>>>
>>>
>>> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
>>> index 85c5a88..dbc14a7 100644
>>> --- a/kernel/rcu/tree_plugin.h
>>> +++ b/kernel/rcu/tree_plugin.h
>>> @@ -2172,7 +2172,7 @@ static int rcu_nocb_kthread(void *arg)
>>> if (__rcu_reclaim(rdp->rsp->name, list))
>>> cl++;
>>> c++;
>>> -   local_bh_enable();
>>> + _local_bh_enable();
>>> cond_resched_rcu_qs();
>>> list = next;
>>> }
>>>
>>>
>>> The cond_resched_rcu_qs() would process the softirq if the softirq is 
>>> pending, so no need to use
>>> local_bh_enable() to process the softirq twice here, and it will avoid OOM 
>>> when huge packets arrives,
>>> what do you think about it? Please give me some suggestion.
>>
>> From what I can see, there is absolutely no guarantee that
>> cond_resched_rcu_qs() will do local_bh_enable(), and thus no guarantee
>> that it will process any pending softirqs -- and that is not part of
>> its job in any case.  So I cannot recommend the above patch.
>>
>> On efficient handling of large invalid packets (that is still the issue,
>> right?), I must defer to Dave and Eric.
> 
> On the perhaps unlikely off-chance that there is a fix for this outside
> of networking, what symptoms are you seeing without this fix in place?
> Still RCU CPU stall warnings?  Soft lockups?  Something else?
> 
>   Thanx, Paul
> 

Hi Paul:

I was still try to test and fix this by another way, but could explain more 
about this problem.

when the huge packets coming, the packets was abnormal and will be freed by 
dst_release->call_rcu(dst_destroy_rcu),
so the rcuos kthread will handle the dst_destroy_rcu to free them, but when the 
rcuos was looping ,I fould the local_bh_enable() will
call do_softirq to receive a certain number of packets which is abnormal and 
need to be free, but more packets is coming so when cond_resched_rcu_qs run,
it will do the ksoftirqd and do softirq again, so rcuos kthread need free more, 
it looks more and more worse and lead to OOM because many more packets need to
be freed.
So I think the do_softirq in the local_bh_enable is not need here, the 
cond_resched_rcu_qs() will handle the do_softirq once, it is enough.

and recently I found that the Eric has upstream a new patch named (softirq: Let 
ksoftirqd do its job) may fix this, and still test it, not get any results yet.

Thanks
Ding



>>> Thanks.
>>> Ding
>>>
>>> On 2016/11/21 9:28, Ding Tianhong wrote:


 On 2016/11/21 8:13, Paul E. McKenney wrote:
> On Sat, Nov 19, 2016 at 12:22:09AM -0800, Paul E. McKenney wrote:
>> On Sat, Nov 19, 2016 at 03:50:32PM +0800, Ding Tianhong wrote:
>>>
>>>
>>> On 2016/11/18 21:01, Paul E. McKenney wrote:
 On Fri, Nov 18, 2016 at 08:40:09PM +0800, Ding Tianhong wrote:
> The commit bedc196915 ("rcu: Fix soft lockup for rcu_nocb_kthread")
> will introduce a new problem that when huge IP abnormal packet 
> arrived,
> it may cause OOM and break the kernel, just like this:
>
> [   79.441538] mlx4_en: eth5: Leaving promiscuous mode steering mode:2
> [  100.067032] ksoftirqd/0: page allocation failure: order:0, 
> mode:0x120
> [  100.067038] CPU: 0 PID: 3 Comm: ksoftirqd/0 Tainted: G   
> OE  V---   3.10.0-327.28.3.28.x86_64 #1
> [  100.067039] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), 
> BIOS rel-1.9.1-0-gb3ef39f-20161018_184732-HGH103483 04/01/2014
> [  100.067041]  0120 b080d798 8802afd5b968 
> 81638cb9
> [  100.067045]  8802afd5b9f8 81171380 0010 
> 
> [  100.067048]  8802befd8000  0001 
> b080d798
> [  100.067050] Call Trace:
> [  100.067057]  [] dump_stack+0x19/0x1b
> [  100.067062]  [] warn_alloc_failed+0x110/0x180
> [  100.067066]  [] 
> __alloc_pages_nodemask+0x9b6/0xba0
> [  100.067070]  [] ? skb_add_rx_frag+0x90/0xb0
> [  100.067075]  [] alloc_pages_current+0xaa/0x170
> [  100.067080]  [] 
> mlx4_alloc_pages.isra.24+0x40/0x170 [mlx4_en]
> [  100.067083]  [] mlx4_en_alloc_frags+0xdc/0x220 
> [mlx4_en]
> [  100.067086]  [] ? __netif_receive_skb+0x18/0x60
> [  100.067088]  [] ? netif_receive_skb+0x40/0xc0
> [  100.067092]  [] 
> 

Re: [PATCH] rcu: fix the OOM problem of huge IP abnormal packet traffic

2017-01-03 Thread Ding Tianhong


On 2017/1/4 8:57, Paul E. McKenney wrote:
> On Wed, Dec 28, 2016 at 04:13:15PM -0800, Paul E. McKenney wrote:
>> On Wed, Dec 28, 2016 at 01:58:06PM +0800, Ding Tianhong wrote:
>>> Hi, Paul:
>>>
>>> I try to debug this problem and found this solution could work well for 
>>> both problem scene.
>>>
>>>
>>> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
>>> index 85c5a88..dbc14a7 100644
>>> --- a/kernel/rcu/tree_plugin.h
>>> +++ b/kernel/rcu/tree_plugin.h
>>> @@ -2172,7 +2172,7 @@ static int rcu_nocb_kthread(void *arg)
>>> if (__rcu_reclaim(rdp->rsp->name, list))
>>> cl++;
>>> c++;
>>> -   local_bh_enable();
>>> + _local_bh_enable();
>>> cond_resched_rcu_qs();
>>> list = next;
>>> }
>>>
>>>
>>> The cond_resched_rcu_qs() would process the softirq if the softirq is 
>>> pending, so no need to use
>>> local_bh_enable() to process the softirq twice here, and it will avoid OOM 
>>> when huge packets arrives,
>>> what do you think about it? Please give me some suggestion.
>>
>> From what I can see, there is absolutely no guarantee that
>> cond_resched_rcu_qs() will do local_bh_enable(), and thus no guarantee
>> that it will process any pending softirqs -- and that is not part of
>> its job in any case.  So I cannot recommend the above patch.
>>
>> On efficient handling of large invalid packets (that is still the issue,
>> right?), I must defer to Dave and Eric.
> 
> On the perhaps unlikely off-chance that there is a fix for this outside
> of networking, what symptoms are you seeing without this fix in place?
> Still RCU CPU stall warnings?  Soft lockups?  Something else?
> 
>   Thanx, Paul
> 

Hi Paul:

I was still try to test and fix this by another way, but could explain more 
about this problem.

when the huge packets coming, the packets was abnormal and will be freed by 
dst_release->call_rcu(dst_destroy_rcu),
so the rcuos kthread will handle the dst_destroy_rcu to free them, but when the 
rcuos was looping ,I fould the local_bh_enable() will
call do_softirq to receive a certain number of packets which is abnormal and 
need to be free, but more packets is coming so when cond_resched_rcu_qs run,
it will do the ksoftirqd and do softirq again, so rcuos kthread need free more, 
it looks more and more worse and lead to OOM because many more packets need to
be freed.
So I think the do_softirq in the local_bh_enable is not need here, the 
cond_resched_rcu_qs() will handle the do_softirq once, it is enough.

and recently I found that the Eric has upstream a new patch named (softirq: Let 
ksoftirqd do its job) may fix this, and still test it, not get any results yet.

Thanks
Ding



>>> Thanks.
>>> Ding
>>>
>>> On 2016/11/21 9:28, Ding Tianhong wrote:


 On 2016/11/21 8:13, Paul E. McKenney wrote:
> On Sat, Nov 19, 2016 at 12:22:09AM -0800, Paul E. McKenney wrote:
>> On Sat, Nov 19, 2016 at 03:50:32PM +0800, Ding Tianhong wrote:
>>>
>>>
>>> On 2016/11/18 21:01, Paul E. McKenney wrote:
 On Fri, Nov 18, 2016 at 08:40:09PM +0800, Ding Tianhong wrote:
> The commit bedc196915 ("rcu: Fix soft lockup for rcu_nocb_kthread")
> will introduce a new problem that when huge IP abnormal packet 
> arrived,
> it may cause OOM and break the kernel, just like this:
>
> [   79.441538] mlx4_en: eth5: Leaving promiscuous mode steering mode:2
> [  100.067032] ksoftirqd/0: page allocation failure: order:0, 
> mode:0x120
> [  100.067038] CPU: 0 PID: 3 Comm: ksoftirqd/0 Tainted: G   
> OE  V---   3.10.0-327.28.3.28.x86_64 #1
> [  100.067039] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), 
> BIOS rel-1.9.1-0-gb3ef39f-20161018_184732-HGH103483 04/01/2014
> [  100.067041]  0120 b080d798 8802afd5b968 
> 81638cb9
> [  100.067045]  8802afd5b9f8 81171380 0010 
> 
> [  100.067048]  8802befd8000  0001 
> b080d798
> [  100.067050] Call Trace:
> [  100.067057]  [] dump_stack+0x19/0x1b
> [  100.067062]  [] warn_alloc_failed+0x110/0x180
> [  100.067066]  [] 
> __alloc_pages_nodemask+0x9b6/0xba0
> [  100.067070]  [] ? skb_add_rx_frag+0x90/0xb0
> [  100.067075]  [] alloc_pages_current+0xaa/0x170
> [  100.067080]  [] 
> mlx4_alloc_pages.isra.24+0x40/0x170 [mlx4_en]
> [  100.067083]  [] mlx4_en_alloc_frags+0xdc/0x220 
> [mlx4_en]
> [  100.067086]  [] ? __netif_receive_skb+0x18/0x60
> [  100.067088]  [] ? netif_receive_skb+0x40/0xc0
> [  100.067092]  [] 
> 

Re: [PATCH v6 06/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_init() to prepare for ACPI

2017-01-03 Thread Hanjun Guo

Hi Tomasz,

On 2017/1/3 15:41, Tomasz Nowicki wrote:

Hi,

Can we merge patch 4 & 6 into one patch so that we keep refactoring part
as one piece ? I do not see a reason to keep them separate or have patch
5 in between. You can refactor what needs to be refactored, add
necessary functions to iort.c and then support ACPI for
irq-gic-v3-its-platform-msi.c


There are two functions here,
 - retrieve the dev id from IORT which was DT based only;

 - init the platform msi domain from MADT;

For each of them split it into two steps,
 - refactor the code for ACPI later and it's easy for review
   because wen can easily to figure out it has functional
   change or not

 - add ACPI functionality

Does it make sense?

Thanks
Hanjun


Re: [PATCH v6 06/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_init() to prepare for ACPI

2017-01-03 Thread Hanjun Guo

Hi Tomasz,

On 2017/1/3 15:41, Tomasz Nowicki wrote:

Hi,

Can we merge patch 4 & 6 into one patch so that we keep refactoring part
as one piece ? I do not see a reason to keep them separate or have patch
5 in between. You can refactor what needs to be refactored, add
necessary functions to iort.c and then support ACPI for
irq-gic-v3-its-platform-msi.c


There are two functions here,
 - retrieve the dev id from IORT which was DT based only;

 - init the platform msi domain from MADT;

For each of them split it into two steps,
 - refactor the code for ACPI later and it's easy for review
   because wen can easily to figure out it has functional
   change or not

 - add ACPI functionality

Does it make sense?

Thanks
Hanjun


[PATCH v4 3/3] arm64: dts: exynos: Add support for S6E3HA2 panel device on TM2 board

2017-01-03 Thread Hoegeun Kwon
From: Hyungwon Hwang 

This patch add the panel device tree node for S6E3HA2 display
controller to TM2 dts.

Signed-off-by: Hyungwon Hwang 
Signed-off-by: Andrzej Hajda 
Signed-off-by: Chanwoo Choi 
Signed-off-by: Hoegeun Kwon 
---
 arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 17 +
 1 file changed, 17 insertions(+)

diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts 
b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
index 5b9451d..b3ba1ac 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
@@ -304,11 +304,28 @@
reg = <1>;
 
dsi_out: endpoint {
+   remote-endpoint = <_in>;
samsung,burst-clock-frequency = <51200>;
samsung,esc-clock-frequency = <1600>;
};
};
};
+
+   panel@0 {
+   compatible = "samsung,s6e3ha2";
+   reg = <0>;
+   vdd3-supply = <_reg>;
+   vci-supply = <_reg>;
+   reset-gpios = < 0 GPIO_ACTIVE_LOW>;
+   enable-gpios = < 5 GPIO_ACTIVE_HIGH>;
+   te-gpios = < 3 GPIO_ACTIVE_HIGH>;
+
+   port {
+   panel_in: endpoint {
+   remote-endpoint = <_out>;
+   };
+   };
+   };
 };
 
 _0 {
-- 
1.9.1



[PATCH v4 3/3] arm64: dts: exynos: Add support for S6E3HA2 panel device on TM2 board

2017-01-03 Thread Hoegeun Kwon
From: Hyungwon Hwang 

This patch add the panel device tree node for S6E3HA2 display
controller to TM2 dts.

Signed-off-by: Hyungwon Hwang 
Signed-off-by: Andrzej Hajda 
Signed-off-by: Chanwoo Choi 
Signed-off-by: Hoegeun Kwon 
---
 arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 17 +
 1 file changed, 17 insertions(+)

diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts 
b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
index 5b9451d..b3ba1ac 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
@@ -304,11 +304,28 @@
reg = <1>;
 
dsi_out: endpoint {
+   remote-endpoint = <_in>;
samsung,burst-clock-frequency = <51200>;
samsung,esc-clock-frequency = <1600>;
};
};
};
+
+   panel@0 {
+   compatible = "samsung,s6e3ha2";
+   reg = <0>;
+   vdd3-supply = <_reg>;
+   vci-supply = <_reg>;
+   reset-gpios = < 0 GPIO_ACTIVE_LOW>;
+   enable-gpios = < 5 GPIO_ACTIVE_HIGH>;
+   te-gpios = < 3 GPIO_ACTIVE_HIGH>;
+
+   port {
+   panel_in: endpoint {
+   remote-endpoint = <_out>;
+   };
+   };
+   };
 };
 
 _0 {
-- 
1.9.1



[PATCH v4 1/3] drm/exynos: mic: Add mode_set callback function

2017-01-03 Thread Hoegeun Kwon
Before applying the patch, used the of_get_videomode function to
parse the display-timings in the panel which is the child driver
of dsi in the devicetree. this is wrong. So removed the
of_get_videomode and fixed to get videomode struct through
mode_set callback function.

Signed-off-by: Hoegeun Kwon 
---
 drivers/gpu/drm/exynos/exynos_drm_mic.c | 33 ++---
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c 
b/drivers/gpu/drm/exynos/exynos_drm_mic.c
index a0def0b..9a50ceb 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_mic.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c
@@ -286,13 +286,6 @@ static int parse_dt(struct exynos_mic *mic)
}
nodes[j++] = remote_node;
 
-   ret = of_get_videomode(remote_node,
-   >vm, 0);
-   if (ret) {
-   DRM_ERROR("mic: failed to get videomode");
-   goto exit;
-   }
-
break;
default:
DRM_ERROR("mic: Unknown endpoint from MIC");
@@ -329,6 +322,27 @@ static void mic_post_disable(struct drm_bridge *bridge)
mutex_unlock(_mutex);
 }
 
+static void mic_mode_set(struct drm_bridge *bridge,
+   struct drm_display_mode *mode,
+   struct drm_display_mode *adjusted_mode)
+{
+   struct exynos_mic *mic = bridge->driver_private;
+
+   mutex_lock(_mutex);
+   if (mic->enabled)
+   goto already_enabled;
+
+   drm_display_mode_to_videomode(mode, >vm);
+
+   if (!mic->i80_mode)
+   mic_set_porch_timing(mic);
+   mic_set_img_size(mic);
+   mic_set_output_timing(mic);
+
+already_enabled:
+   mutex_unlock(_mutex);
+}
+
 static void mic_pre_enable(struct drm_bridge *bridge)
 {
struct exynos_mic *mic = bridge->driver_private;
@@ -355,10 +369,6 @@ static void mic_pre_enable(struct drm_bridge *bridge)
goto turn_off_clks;
}
 
-   if (!mic->i80_mode)
-   mic_set_porch_timing(mic);
-   mic_set_img_size(mic);
-   mic_set_output_timing(mic);
mic_set_reg_on(mic, 1);
mic->enabled = 1;
mutex_unlock(_mutex);
@@ -377,6 +387,7 @@ static void mic_enable(struct drm_bridge *bridge) { }
 static const struct drm_bridge_funcs mic_bridge_funcs = {
.disable = mic_disable,
.post_disable = mic_post_disable,
+   .mode_set = mic_mode_set,
.pre_enable = mic_pre_enable,
.enable = mic_enable,
 };
-- 
1.9.1



[PATCH v4 1/3] drm/exynos: mic: Add mode_set callback function

2017-01-03 Thread Hoegeun Kwon
Before applying the patch, used the of_get_videomode function to
parse the display-timings in the panel which is the child driver
of dsi in the devicetree. this is wrong. So removed the
of_get_videomode and fixed to get videomode struct through
mode_set callback function.

Signed-off-by: Hoegeun Kwon 
---
 drivers/gpu/drm/exynos/exynos_drm_mic.c | 33 ++---
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c 
b/drivers/gpu/drm/exynos/exynos_drm_mic.c
index a0def0b..9a50ceb 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_mic.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c
@@ -286,13 +286,6 @@ static int parse_dt(struct exynos_mic *mic)
}
nodes[j++] = remote_node;
 
-   ret = of_get_videomode(remote_node,
-   >vm, 0);
-   if (ret) {
-   DRM_ERROR("mic: failed to get videomode");
-   goto exit;
-   }
-
break;
default:
DRM_ERROR("mic: Unknown endpoint from MIC");
@@ -329,6 +322,27 @@ static void mic_post_disable(struct drm_bridge *bridge)
mutex_unlock(_mutex);
 }
 
+static void mic_mode_set(struct drm_bridge *bridge,
+   struct drm_display_mode *mode,
+   struct drm_display_mode *adjusted_mode)
+{
+   struct exynos_mic *mic = bridge->driver_private;
+
+   mutex_lock(_mutex);
+   if (mic->enabled)
+   goto already_enabled;
+
+   drm_display_mode_to_videomode(mode, >vm);
+
+   if (!mic->i80_mode)
+   mic_set_porch_timing(mic);
+   mic_set_img_size(mic);
+   mic_set_output_timing(mic);
+
+already_enabled:
+   mutex_unlock(_mutex);
+}
+
 static void mic_pre_enable(struct drm_bridge *bridge)
 {
struct exynos_mic *mic = bridge->driver_private;
@@ -355,10 +369,6 @@ static void mic_pre_enable(struct drm_bridge *bridge)
goto turn_off_clks;
}
 
-   if (!mic->i80_mode)
-   mic_set_porch_timing(mic);
-   mic_set_img_size(mic);
-   mic_set_output_timing(mic);
mic_set_reg_on(mic, 1);
mic->enabled = 1;
mutex_unlock(_mutex);
@@ -377,6 +387,7 @@ static void mic_enable(struct drm_bridge *bridge) { }
 static const struct drm_bridge_funcs mic_bridge_funcs = {
.disable = mic_disable,
.post_disable = mic_post_disable,
+   .mode_set = mic_mode_set,
.pre_enable = mic_pre_enable,
.enable = mic_enable,
 };
-- 
1.9.1



[PATCH v4 0/3] Add support for the S6E3HA2 panel on TM2 board

2017-01-03 Thread Hoegeun Kwon
Purpose of this patch is add support for S6E3HA2 AMOLED panel on
the TM2 board. The first patch adds support for S6E3HA2 panel
device tree document and driver, the second patch add support for
S6E3HA2 panel device tree.

Changes for V4:

- Removed display-timings in devicetree, the display-timings has
  been fixed to be provided by the device driver.
- Added the mode_set callback function into exynos_drm_mic,
  because the exynos_drm_mic driver can not parse a videomode
  struct by removing the display-timings from the devicetree.

Hoegeun Kwon (2):
  drm/exynos: mic: Add mode_set callback function
  drm/panel: Add support for S6E3HA2 panel driver on TM2 board

Hyungwon Hwang (1):
  arm64: dts: exynos: Add support for S6E3HA2 panel device on TM2 board

 .../bindings/display/panel/samsung,s6e3ha2.txt |  40 ++
 arch/arm64/boot/dts/exynos/exynos5433-tm2.dts  |  17 +
 drivers/gpu/drm/exynos/exynos_drm_mic.c|  33 +-
 drivers/gpu/drm/panel/Kconfig  |   6 +
 drivers/gpu/drm/panel/Makefile |   1 +
 drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c  | 741 +
 6 files changed, 827 insertions(+), 11 deletions(-)
 create mode 100644 
Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt
 create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c

-- 
1.9.1



[PATCH v4 2/3] drm/panel: Add support for S6E3HA2 panel driver on TM2 board

2017-01-03 Thread Hoegeun Kwon
This patch add support for MIPI-DSI based S6E3HA2 AMOLED panel
driver. This panel has 1440x2560 resolution in 5.7-inch physical
panel in the TM2 device.

Signed-off-by: Donghwa Lee 
Signed-off-by: Hyungwon Hwang 
Signed-off-by: Hoegeun Kwon 
---
 .../bindings/display/panel/samsung,s6e3ha2.txt |  40 ++
 drivers/gpu/drm/panel/Kconfig  |   6 +
 drivers/gpu/drm/panel/Makefile |   1 +
 drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c  | 741 +
 4 files changed, 788 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt
 create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c

diff --git 
a/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt 
b/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt
new file mode 100644
index 000..6879f51
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt
@@ -0,0 +1,40 @@
+Samsung S6E3HA2 5.7" 1440x2560 AMOLED panel
+
+Required properties:
+  - compatible: "samsung,s6e3ha2"
+  - reg: the virtual channel number of a DSI peripheral
+  - vdd3-supply: I/O voltage supply
+  - vci-supply: voltage supply for analog circuits
+  - reset-gpios: a GPIO spec for the reset pin (active low)
+  - enable-gpios: a GPIO spec for the panel enable pin (active high)
+  - te-gpios: a GPIO spec for the tearing effect synchronization signal
+gpio pin (active high)
+
+The device node can contain one 'port' child node with one child
+'endpoint' node, according to the bindings defined in [1]. This
+node should describe panel's video bus.
+
+[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
+
+Example:
+
+ {
+   ...
+
+   panel@0 {
+   compatible = "samsung,s6e3ha2";
+   reg = <0>;
+   vdd3-supply = <_reg>;
+   vci-supply = <_reg>;
+   reset-gpios = < 0 GPIO_ACTIVE_LOW>;
+   enable-gpios = < 5 GPIO_ACTIVE_HIGH>;
+   te-gpios = < 3 GPIO_ACTIVE_HIGH>;
+
+   port {
+   panel_in: endpoint {
+   remote-endpoint = <_out>;
+   };
+   };
+   };
+};
+
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 62aba97..eea2902 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -52,6 +52,12 @@ config DRM_PANEL_PANASONIC_VVX10F034N00
  WUXGA (1920x1200) Novatek NT1397-based DSI panel as found in some
  Xperia Z2 tablets
 
+config DRM_PANEL_SAMSUNG_S6E3HA2
+   tristate "Samsung S6E3HA2 DSI video mode panel"
+   depends on OF
+   depends on  DRM_MIPI_DSI
+   select VIDEOMODE_HELPERS
+
 config DRM_PANEL_SAMSUNG_S6E8AA0
tristate "Samsung S6E8AA0 DSI video mode panel"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index a5c7ec0..1d483b0 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += 
panel-jdi-lt070me05000.o
 obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
 obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) += 
panel-panasonic-vvx10f034n00.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
+obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA2) += panel-samsung-s6e3ha2.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0) += panel-samsung-s6e8aa0.o
 obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
 obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c 
b/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
new file mode 100644
index 000..8c5a1c2
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
@@ -0,0 +1,741 @@
+/*
+ * MIPI-DSI based s6e3ha2 AMOLED 5.7 inch panel driver.
+ *
+ * Copyright (c) 2016 Samsung Electronics Co., Ltd.
+ * Donghwa Lee 
+ * Hyungwon Hwang 
+ * Hoegeun Kwon 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define S6E3HA2_MIN_BRIGHTNESS 0
+#define S6E3HA2_MAX_BRIGHTNESS 100
+#define S6E3HA2_DEFAULT_BRIGHTNESS 80
+
+#define S6E3HA2_NUM_GAMMA_STEPS46
+#define S6E3HA2_GAMMA_CMD_CNT  35
+#define S6E3HA2_VINT_STATUS_MAX10
+
+static const u8 gamma_tbl[S6E3HA2_NUM_GAMMA_STEPS][S6E3HA2_GAMMA_CMD_CNT] = {
+   { 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 0x87, 0x87, 0x82, 0x83,
+ 0x85, 0x88, 0x8b, 0x8b, 0x84, 0x88, 0x82, 0x82, 0x89, 0x86, 0x8c,
+  

[PATCH v4 0/3] Add support for the S6E3HA2 panel on TM2 board

2017-01-03 Thread Hoegeun Kwon
Purpose of this patch is add support for S6E3HA2 AMOLED panel on
the TM2 board. The first patch adds support for S6E3HA2 panel
device tree document and driver, the second patch add support for
S6E3HA2 panel device tree.

Changes for V4:

- Removed display-timings in devicetree, the display-timings has
  been fixed to be provided by the device driver.
- Added the mode_set callback function into exynos_drm_mic,
  because the exynos_drm_mic driver can not parse a videomode
  struct by removing the display-timings from the devicetree.

Hoegeun Kwon (2):
  drm/exynos: mic: Add mode_set callback function
  drm/panel: Add support for S6E3HA2 panel driver on TM2 board

Hyungwon Hwang (1):
  arm64: dts: exynos: Add support for S6E3HA2 panel device on TM2 board

 .../bindings/display/panel/samsung,s6e3ha2.txt |  40 ++
 arch/arm64/boot/dts/exynos/exynos5433-tm2.dts  |  17 +
 drivers/gpu/drm/exynos/exynos_drm_mic.c|  33 +-
 drivers/gpu/drm/panel/Kconfig  |   6 +
 drivers/gpu/drm/panel/Makefile |   1 +
 drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c  | 741 +
 6 files changed, 827 insertions(+), 11 deletions(-)
 create mode 100644 
Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt
 create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c

-- 
1.9.1



[PATCH v4 2/3] drm/panel: Add support for S6E3HA2 panel driver on TM2 board

2017-01-03 Thread Hoegeun Kwon
This patch add support for MIPI-DSI based S6E3HA2 AMOLED panel
driver. This panel has 1440x2560 resolution in 5.7-inch physical
panel in the TM2 device.

Signed-off-by: Donghwa Lee 
Signed-off-by: Hyungwon Hwang 
Signed-off-by: Hoegeun Kwon 
---
 .../bindings/display/panel/samsung,s6e3ha2.txt |  40 ++
 drivers/gpu/drm/panel/Kconfig  |   6 +
 drivers/gpu/drm/panel/Makefile |   1 +
 drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c  | 741 +
 4 files changed, 788 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt
 create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c

diff --git 
a/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt 
b/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt
new file mode 100644
index 000..6879f51
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt
@@ -0,0 +1,40 @@
+Samsung S6E3HA2 5.7" 1440x2560 AMOLED panel
+
+Required properties:
+  - compatible: "samsung,s6e3ha2"
+  - reg: the virtual channel number of a DSI peripheral
+  - vdd3-supply: I/O voltage supply
+  - vci-supply: voltage supply for analog circuits
+  - reset-gpios: a GPIO spec for the reset pin (active low)
+  - enable-gpios: a GPIO spec for the panel enable pin (active high)
+  - te-gpios: a GPIO spec for the tearing effect synchronization signal
+gpio pin (active high)
+
+The device node can contain one 'port' child node with one child
+'endpoint' node, according to the bindings defined in [1]. This
+node should describe panel's video bus.
+
+[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
+
+Example:
+
+ {
+   ...
+
+   panel@0 {
+   compatible = "samsung,s6e3ha2";
+   reg = <0>;
+   vdd3-supply = <_reg>;
+   vci-supply = <_reg>;
+   reset-gpios = < 0 GPIO_ACTIVE_LOW>;
+   enable-gpios = < 5 GPIO_ACTIVE_HIGH>;
+   te-gpios = < 3 GPIO_ACTIVE_HIGH>;
+
+   port {
+   panel_in: endpoint {
+   remote-endpoint = <_out>;
+   };
+   };
+   };
+};
+
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 62aba97..eea2902 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -52,6 +52,12 @@ config DRM_PANEL_PANASONIC_VVX10F034N00
  WUXGA (1920x1200) Novatek NT1397-based DSI panel as found in some
  Xperia Z2 tablets
 
+config DRM_PANEL_SAMSUNG_S6E3HA2
+   tristate "Samsung S6E3HA2 DSI video mode panel"
+   depends on OF
+   depends on  DRM_MIPI_DSI
+   select VIDEOMODE_HELPERS
+
 config DRM_PANEL_SAMSUNG_S6E8AA0
tristate "Samsung S6E8AA0 DSI video mode panel"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index a5c7ec0..1d483b0 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += 
panel-jdi-lt070me05000.o
 obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
 obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) += 
panel-panasonic-vvx10f034n00.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
+obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA2) += panel-samsung-s6e3ha2.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0) += panel-samsung-s6e8aa0.o
 obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
 obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c 
b/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
new file mode 100644
index 000..8c5a1c2
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
@@ -0,0 +1,741 @@
+/*
+ * MIPI-DSI based s6e3ha2 AMOLED 5.7 inch panel driver.
+ *
+ * Copyright (c) 2016 Samsung Electronics Co., Ltd.
+ * Donghwa Lee 
+ * Hyungwon Hwang 
+ * Hoegeun Kwon 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define S6E3HA2_MIN_BRIGHTNESS 0
+#define S6E3HA2_MAX_BRIGHTNESS 100
+#define S6E3HA2_DEFAULT_BRIGHTNESS 80
+
+#define S6E3HA2_NUM_GAMMA_STEPS46
+#define S6E3HA2_GAMMA_CMD_CNT  35
+#define S6E3HA2_VINT_STATUS_MAX10
+
+static const u8 gamma_tbl[S6E3HA2_NUM_GAMMA_STEPS][S6E3HA2_GAMMA_CMD_CNT] = {
+   { 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 0x87, 0x87, 0x82, 0x83,
+ 0x85, 0x88, 0x8b, 0x8b, 0x84, 0x88, 0x82, 0x82, 0x89, 0x86, 0x8c,
+ 0x94, 0x84, 0xb1, 0xaf, 0x8e, 0xcf, 0xad, 0xc9, 0x00, 0x00, 0x00,
+ 0x00, 0x00 },
+   { 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 

[Acked] [PATCH] checkpatch: Warn on embedded function names

2017-01-03 Thread Andy Whitcroft
On Tue, Jan 03, 2017 at 07:21:54PM -0800, Joe Perches wrote:
> Embedded function names are less appropriate to use when
> refactoring can cause function renaming.  Prefer the use
> of "%s", __func__ to embedded function names.
> 
> Signed-off-by: Joe Perches 
> ---
>  scripts/checkpatch.pl | 22 --
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 982c52ca6473..4f53093a1b0b 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2134,7 +2134,7 @@ sub process {
>   my $in_header_lines = $file ? 0 : 1;
>   my $in_commit_log = 0;  #Scanning lines before patch
>   my $has_commit_log = 0; #Encountered lines before patch
> -   my $commit_log_possible_stack_dump = 0;
> + my $commit_log_possible_stack_dump = 0;
>   my $commit_log_long_line = 0;
>   my $commit_log_has_diff = 0;
>   my $reported_maintainer_file = 0;
> @@ -2154,6 +2154,7 @@ sub process {
>   my $realline = 0;
>   my $realcnt = 0;
>   my $here = '';
> + my $context_function;   #undef'd unless there's a known function
>   my $in_comment = 0;
>   my $comment_edge = 0;
>   my $first_line = 0;
> @@ -2192,7 +2193,8 @@ sub process {
>   }
>   #next;
>   }
> - if ($rawline=~/^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@/) {
> + if ($rawline=~/^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@(.*)/) {
> + my $context = $4;
>   $realline=$1-1;
>   if (defined $2) {
>   $realcnt=$3+1;
> @@ -2201,6 +2203,12 @@ sub process {
>   }
>   $in_comment = 0;
>  
> + if ($context =~ /\b(\w+)\s*\(/) {
> + $context_function = $1;
> + } else {
> + undef $context_function;
> + }
> +
>   # Guestimate if this is a continuing comment.  Run
>   # the context looking for a comment "edge".  If this
>   # edge is a close comment then we must be in a comment
> @@ -5157,6 +5165,16 @@ sub process {
>"break quoted strings at a space character\n" . 
> $hereprev);
>   }
>  
> +#check for an embedded function name in a string when the function is known
> +# as part of a diff.  This does not work for -f --file checking as it
> +#depends on patch context providing the function name
> + if ($line =~ /^\+.*$String/ &&
> + defined($context_function) &&
> + get_quoted_string($line, $rawline) =~ 
> /\b$context_function\b/) {
> + WARN("EMBEDDED_FUNCTION_NAME",
> +  "Prefer using \"%s\", __func__ to embedded 
> function names\n" . $herecurr);
> + }
> +
>  # check for spaces before a quoted newline
>   if ($rawline =~ /^.*\".*\s\\n/) {
>   if (WARN("QUOTED_WHITESPACE_BEFORE_NEWLINE",
> -- 
> 2.10.0.rc2.1.g053435c
> 

Looks sane enough to me.

Acked-by: Andy Whitcroft 

-apw


[Acked] [PATCH] checkpatch: Warn on embedded function names

2017-01-03 Thread Andy Whitcroft
On Tue, Jan 03, 2017 at 07:21:54PM -0800, Joe Perches wrote:
> Embedded function names are less appropriate to use when
> refactoring can cause function renaming.  Prefer the use
> of "%s", __func__ to embedded function names.
> 
> Signed-off-by: Joe Perches 
> ---
>  scripts/checkpatch.pl | 22 --
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 982c52ca6473..4f53093a1b0b 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2134,7 +2134,7 @@ sub process {
>   my $in_header_lines = $file ? 0 : 1;
>   my $in_commit_log = 0;  #Scanning lines before patch
>   my $has_commit_log = 0; #Encountered lines before patch
> -   my $commit_log_possible_stack_dump = 0;
> + my $commit_log_possible_stack_dump = 0;
>   my $commit_log_long_line = 0;
>   my $commit_log_has_diff = 0;
>   my $reported_maintainer_file = 0;
> @@ -2154,6 +2154,7 @@ sub process {
>   my $realline = 0;
>   my $realcnt = 0;
>   my $here = '';
> + my $context_function;   #undef'd unless there's a known function
>   my $in_comment = 0;
>   my $comment_edge = 0;
>   my $first_line = 0;
> @@ -2192,7 +2193,8 @@ sub process {
>   }
>   #next;
>   }
> - if ($rawline=~/^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@/) {
> + if ($rawline=~/^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@(.*)/) {
> + my $context = $4;
>   $realline=$1-1;
>   if (defined $2) {
>   $realcnt=$3+1;
> @@ -2201,6 +2203,12 @@ sub process {
>   }
>   $in_comment = 0;
>  
> + if ($context =~ /\b(\w+)\s*\(/) {
> + $context_function = $1;
> + } else {
> + undef $context_function;
> + }
> +
>   # Guestimate if this is a continuing comment.  Run
>   # the context looking for a comment "edge".  If this
>   # edge is a close comment then we must be in a comment
> @@ -5157,6 +5165,16 @@ sub process {
>"break quoted strings at a space character\n" . 
> $hereprev);
>   }
>  
> +#check for an embedded function name in a string when the function is known
> +# as part of a diff.  This does not work for -f --file checking as it
> +#depends on patch context providing the function name
> + if ($line =~ /^\+.*$String/ &&
> + defined($context_function) &&
> + get_quoted_string($line, $rawline) =~ 
> /\b$context_function\b/) {
> + WARN("EMBEDDED_FUNCTION_NAME",
> +  "Prefer using \"%s\", __func__ to embedded 
> function names\n" . $herecurr);
> + }
> +
>  # check for spaces before a quoted newline
>   if ($rawline =~ /^.*\".*\s\\n/) {
>   if (WARN("QUOTED_WHITESPACE_BEFORE_NEWLINE",
> -- 
> 2.10.0.rc2.1.g053435c
> 

Looks sane enough to me.

Acked-by: Andy Whitcroft 

-apw


Re: [PATCH 7/7] misc: intel-ish-client: add intel ishtp clients driver

2017-01-03 Thread Xu, Even
On 二, 2017-01-03 at 10:54 +0100, Jiri Kosina wrote:
> On Fri, 23 Dec 2016, Even Xu wrote:
> 
> [ ... snip ... ]
> > 
> > +static ssize_t ishtp_cl_write(struct file *file, const char __user
> > *ubuf,
> > +   size_t length, loff_t *offset)
> > +{
> > +   struct ishtp_cl_miscdev *ishtp_cl_misc = file-
> > >private_data;
> > +   struct ishtp_cl *cl;
> > +   void *write_buf;
> > +   struct ishtp_device *dev;
> > +   int ret;
> > +
> > +   /* Non-blocking semantics are not supported */
> > +   if (file->f_flags & O_NONBLOCK) {
> > +   ret = -EINVAL;
> > +   goto out_unlock;
> When taking the error path here you'd try to unlock 
> ishtp_cl_misc->cl_mutex before actually acquiring it.
> 

Thanks for your comments, Jiri, I update my patch below:
===

Intel ISHFW supports many different clients, in
hid/intel-ish-hid/ishtp bus driver, it creates following client devices:
HID client:
interface of sensor configure and sensor event report.
SMHI client:
interface of sensor calibration, ISHFW debug, ISHFW performance
analysis and manufacture support.
Trace client:
interface of ISHFW debug log output.
Trace configure client:
interface of ISHFW debug log configuration, such as output port,
log level, filter.
ISHFW loader client:
interface of customized ISHFW loader.
HID client has been handle by hid/intel-ish-hid/intel-ishtp-hid client
driver, and rest of the clients export interface using miscellaneous
drivers. This interface is used by user space tools for debugging and
calibration of sensors.

Signed-off-by: Even Xu 
Reviewed-by: Andriy Shevchenko 
Reviewed-by: Srinivas Pandruvada 
---
 drivers/misc/Kconfig   |   1 +
 drivers/misc/Makefile  |   1 +
 drivers/misc/intel-ish-client/Kconfig  |  15 +
 drivers/misc/intel-ish-client/Makefile |   8 +
 .../misc/intel-ish-client/intel-ishtp-clients.c| 882 +
 include/uapi/linux/intel-ishtp-clients.h   |  73 ++
 6 files changed, 980 insertions(+)
 create mode 100644 drivers/misc/intel-ish-client/Kconfig
 create mode 100644 drivers/misc/intel-ish-client/Makefile
 create mode 100644 drivers/misc/intel-ish-client/intel-ishtp-clients.c
 create mode 100644 include/uapi/linux/intel-ishtp-clients.h

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 64971ba..a89849f 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -778,4 +778,5 @@ source "drivers/misc/mic/Kconfig"
 source "drivers/misc/genwqe/Kconfig"
 source "drivers/misc/echo/Kconfig"
 source "drivers/misc/cxl/Kconfig"
+source "drivers/misc/intel-ish-client/Kconfig"
 endmenu
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 3198336..c54015d 100644
--- a/drivers/misc/Makefile

+++ b/drivers/misc/Makefile
@@ -53,6 +53,7 @@ obj-$(CONFIG_ECHO)+= echo/
 obj-$(CONFIG_VEXPRESS_SYSCFG)  += vexpress-syscfg.o
 obj-$(CONFIG_CXL_BASE) += cxl/
 obj-$(CONFIG_PANEL) += panel.o
+obj-$(CONFIG_INTEL_ISH_CLIENT) += intel-ish-client/
 
 lkdtm-$(CONFIG_LKDTM)  += lkdtm_core.o
 lkdtm-$(CONFIG_LKDTM)  += lkdtm_bugs.o
diff --git a/drivers/misc/intel-ish-client/Kconfig 
b/drivers/misc/intel-ish-client/Kconfig
new file mode 100644
index 000..6fa9cc0
--- /dev/null
+++ b/drivers/misc/intel-ish-client/Kconfig
@@ -0,0 +1,15 @@
+menu "Intel ISH Client support"
+   depends on INTEL_ISH_HID
+
+config INTEL_ISH_CLIENT
+   tristate "Intel Integrated Sensor Hub Client"
+   help
+     The Integrated Sensor Hub (ISH) supports many clients, Intel ISH 
client
+     driver supports following clients:
+     SMHI client: calibration & perfermance & menufacture tool interface
+     trace configure client: ISHFW trace parameter configure interface
+     trace tool client: ISHFW trace log output interface
+     loader client: loading customized ISHFW interface
+
+     Say Y here if you want to support Intel ISH clients. If unsure, say N.
+endmenu
diff --git a/drivers/misc/intel-ish-client/Makefile 
b/drivers/misc/intel-ish-client/Makefile
new file mode 100644
index 000..29a5461
--- /dev/null
+++ b/drivers/misc/intel-ish-client/Makefile
@@ -0,0 +1,8 @@
+#
+# Makefile - Intel ISH client driver
+# Copyright (c) 2014-2016, Intel Corporation.
+#
+#
+obj-$(CONFIG_INTEL_ISH_CLIENT) += intel-ishtp-clients.o
+
+ccflags-y += -I$(srctree)/drivers/hid/intel-ish-hid/ishtp
diff --git a/drivers/misc/intel-ish-client/intel-ishtp-clients.c 
b/drivers/misc/intel-ish-client/intel-ishtp-clients.c
new file mode 100644
index 000..4ca3ab8
--- /dev/null
+++ b/drivers/misc/intel-ish-client/intel-ishtp-clients.c
@@ -0,0 +1,882 @@
+/*
+ * ISHTP clients driver
+ *
+ * Copyright (c) 2016, Intel Corporation.
+ *
+ * This program is free software; 

Re: [PATCH 7/7] misc: intel-ish-client: add intel ishtp clients driver

2017-01-03 Thread Xu, Even
On 二, 2017-01-03 at 10:54 +0100, Jiri Kosina wrote:
> On Fri, 23 Dec 2016, Even Xu wrote:
> 
> [ ... snip ... ]
> > 
> > +static ssize_t ishtp_cl_write(struct file *file, const char __user
> > *ubuf,
> > +   size_t length, loff_t *offset)
> > +{
> > +   struct ishtp_cl_miscdev *ishtp_cl_misc = file-
> > >private_data;
> > +   struct ishtp_cl *cl;
> > +   void *write_buf;
> > +   struct ishtp_device *dev;
> > +   int ret;
> > +
> > +   /* Non-blocking semantics are not supported */
> > +   if (file->f_flags & O_NONBLOCK) {
> > +   ret = -EINVAL;
> > +   goto out_unlock;
> When taking the error path here you'd try to unlock 
> ishtp_cl_misc->cl_mutex before actually acquiring it.
> 

Thanks for your comments, Jiri, I update my patch below:
===

Intel ISHFW supports many different clients, in
hid/intel-ish-hid/ishtp bus driver, it creates following client devices:
HID client:
interface of sensor configure and sensor event report.
SMHI client:
interface of sensor calibration, ISHFW debug, ISHFW performance
analysis and manufacture support.
Trace client:
interface of ISHFW debug log output.
Trace configure client:
interface of ISHFW debug log configuration, such as output port,
log level, filter.
ISHFW loader client:
interface of customized ISHFW loader.
HID client has been handle by hid/intel-ish-hid/intel-ishtp-hid client
driver, and rest of the clients export interface using miscellaneous
drivers. This interface is used by user space tools for debugging and
calibration of sensors.

Signed-off-by: Even Xu 
Reviewed-by: Andriy Shevchenko 
Reviewed-by: Srinivas Pandruvada 
---
 drivers/misc/Kconfig   |   1 +
 drivers/misc/Makefile  |   1 +
 drivers/misc/intel-ish-client/Kconfig  |  15 +
 drivers/misc/intel-ish-client/Makefile |   8 +
 .../misc/intel-ish-client/intel-ishtp-clients.c| 882 +
 include/uapi/linux/intel-ishtp-clients.h   |  73 ++
 6 files changed, 980 insertions(+)
 create mode 100644 drivers/misc/intel-ish-client/Kconfig
 create mode 100644 drivers/misc/intel-ish-client/Makefile
 create mode 100644 drivers/misc/intel-ish-client/intel-ishtp-clients.c
 create mode 100644 include/uapi/linux/intel-ishtp-clients.h

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 64971ba..a89849f 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -778,4 +778,5 @@ source "drivers/misc/mic/Kconfig"
 source "drivers/misc/genwqe/Kconfig"
 source "drivers/misc/echo/Kconfig"
 source "drivers/misc/cxl/Kconfig"
+source "drivers/misc/intel-ish-client/Kconfig"
 endmenu
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 3198336..c54015d 100644
--- a/drivers/misc/Makefile

+++ b/drivers/misc/Makefile
@@ -53,6 +53,7 @@ obj-$(CONFIG_ECHO)+= echo/
 obj-$(CONFIG_VEXPRESS_SYSCFG)  += vexpress-syscfg.o
 obj-$(CONFIG_CXL_BASE) += cxl/
 obj-$(CONFIG_PANEL) += panel.o
+obj-$(CONFIG_INTEL_ISH_CLIENT) += intel-ish-client/
 
 lkdtm-$(CONFIG_LKDTM)  += lkdtm_core.o
 lkdtm-$(CONFIG_LKDTM)  += lkdtm_bugs.o
diff --git a/drivers/misc/intel-ish-client/Kconfig 
b/drivers/misc/intel-ish-client/Kconfig
new file mode 100644
index 000..6fa9cc0
--- /dev/null
+++ b/drivers/misc/intel-ish-client/Kconfig
@@ -0,0 +1,15 @@
+menu "Intel ISH Client support"
+   depends on INTEL_ISH_HID
+
+config INTEL_ISH_CLIENT
+   tristate "Intel Integrated Sensor Hub Client"
+   help
+     The Integrated Sensor Hub (ISH) supports many clients, Intel ISH 
client
+     driver supports following clients:
+     SMHI client: calibration & perfermance & menufacture tool interface
+     trace configure client: ISHFW trace parameter configure interface
+     trace tool client: ISHFW trace log output interface
+     loader client: loading customized ISHFW interface
+
+     Say Y here if you want to support Intel ISH clients. If unsure, say N.
+endmenu
diff --git a/drivers/misc/intel-ish-client/Makefile 
b/drivers/misc/intel-ish-client/Makefile
new file mode 100644
index 000..29a5461
--- /dev/null
+++ b/drivers/misc/intel-ish-client/Makefile
@@ -0,0 +1,8 @@
+#
+# Makefile - Intel ISH client driver
+# Copyright (c) 2014-2016, Intel Corporation.
+#
+#
+obj-$(CONFIG_INTEL_ISH_CLIENT) += intel-ishtp-clients.o
+
+ccflags-y += -I$(srctree)/drivers/hid/intel-ish-hid/ishtp
diff --git a/drivers/misc/intel-ish-client/intel-ishtp-clients.c 
b/drivers/misc/intel-ish-client/intel-ishtp-clients.c
new file mode 100644
index 000..4ca3ab8
--- /dev/null
+++ b/drivers/misc/intel-ish-client/intel-ishtp-clients.c
@@ -0,0 +1,882 @@
+/*
+ * ISHTP clients driver
+ *
+ * Copyright (c) 2016, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU 

RE: [PATCH 1/1] regulator: fixed: add suspend pm routines for pinctrl

2017-01-03 Thread Peter Chen
>On Tue, Jan 03, 2017 at 05:02:47PM +0800, Peter Chen wrote:
>> At some systems, the pinctrl setting will be lost or needs to set as
>> "sleep" state to save power consumption. So, we need to configure
>> pinctrl as "sleep" state when system enters suspend, and as "default"
>> state after system resumes. In this way, the pinctrl value can be
>> recovered as "default" state after resuming.
>
>I thought this was supposed to be done automatically by the driver core?
>If not it should be, every single driver is likely to end up with that.

Good idea, I will send a patch for that.

Peter


RE: [PATCH 1/1] regulator: fixed: add suspend pm routines for pinctrl

2017-01-03 Thread Peter Chen
>On Tue, Jan 03, 2017 at 05:02:47PM +0800, Peter Chen wrote:
>> At some systems, the pinctrl setting will be lost or needs to set as
>> "sleep" state to save power consumption. So, we need to configure
>> pinctrl as "sleep" state when system enters suspend, and as "default"
>> state after system resumes. In this way, the pinctrl value can be
>> recovered as "default" state after resuming.
>
>I thought this was supposed to be done automatically by the driver core?
>If not it should be, every single driver is likely to end up with that.

Good idea, I will send a patch for that.

Peter


RE: [PATCH 1/1] extcon: usb-gpio: add pinctrl operation during system PM

2017-01-03 Thread Peter Chen
 
>
>On 03/01/17 10:17, Peter Chen wrote:
>> At some systems, the pinctrl setting will be lost or needs to set as
>> "sleep" state to save power consumption. So, we need to configure
>> pinctrl as "sleep" state when system enters suspend, and as "default"
>> state after system resumes. In this way, the pinctrl value can be
>> recovered as "default" state after resuming.
>>
>> Signed-off-by: Peter Chen 
>> ---
>>  drivers/extcon/extcon-usb-gpio.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/extcon/extcon-usb-gpio.c
>> b/drivers/extcon/extcon-usb-gpio.c
>> index d589c5f..f7172ae 100644
>> --- a/drivers/extcon/extcon-usb-gpio.c
>> +++ b/drivers/extcon/extcon-usb-gpio.c
>> @@ -244,6 +244,7 @@ static int usb_extcon_suspend(struct device *dev)
>>  disable_irq(info->id_irq);
>>  if (info->vbus_gpiod)
>>  disable_irq(info->vbus_irq);
>> +pinctrl_pm_select_sleep_state(dev);
>>
>>  return ret;
>>  }
>> @@ -253,6 +254,7 @@ static int usb_extcon_resume(struct device *dev)
>>  struct usb_extcon_info *info = dev_get_drvdata(dev);
>>  int ret = 0;
>>
>> +pinctrl_pm_select_default_state(dev);
>>  if (device_may_wakeup(dev)) {
>>  if (info->id_gpiod) {
>>  ret = disable_irq_wake(info->id_irq);
>>
>
>How does this work if wake from suspend is desired?
>pinctrl sleep state might not support wakeup.
>

Oh, yes. I should consider wakeup condition. For GPIO pins,
if wakeup is requested by user, the pinctrl should NOT in
sleep state.

Peter


RE: [PATCH 1/1] extcon: usb-gpio: add pinctrl operation during system PM

2017-01-03 Thread Peter Chen
 
>
>On 03/01/17 10:17, Peter Chen wrote:
>> At some systems, the pinctrl setting will be lost or needs to set as
>> "sleep" state to save power consumption. So, we need to configure
>> pinctrl as "sleep" state when system enters suspend, and as "default"
>> state after system resumes. In this way, the pinctrl value can be
>> recovered as "default" state after resuming.
>>
>> Signed-off-by: Peter Chen 
>> ---
>>  drivers/extcon/extcon-usb-gpio.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/extcon/extcon-usb-gpio.c
>> b/drivers/extcon/extcon-usb-gpio.c
>> index d589c5f..f7172ae 100644
>> --- a/drivers/extcon/extcon-usb-gpio.c
>> +++ b/drivers/extcon/extcon-usb-gpio.c
>> @@ -244,6 +244,7 @@ static int usb_extcon_suspend(struct device *dev)
>>  disable_irq(info->id_irq);
>>  if (info->vbus_gpiod)
>>  disable_irq(info->vbus_irq);
>> +pinctrl_pm_select_sleep_state(dev);
>>
>>  return ret;
>>  }
>> @@ -253,6 +254,7 @@ static int usb_extcon_resume(struct device *dev)
>>  struct usb_extcon_info *info = dev_get_drvdata(dev);
>>  int ret = 0;
>>
>> +pinctrl_pm_select_default_state(dev);
>>  if (device_may_wakeup(dev)) {
>>  if (info->id_gpiod) {
>>  ret = disable_irq_wake(info->id_irq);
>>
>
>How does this work if wake from suspend is desired?
>pinctrl sleep state might not support wakeup.
>

Oh, yes. I should consider wakeup condition. For GPIO pins,
if wakeup is requested by user, the pinctrl should NOT in
sleep state.

Peter


Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on Dell machines

2017-01-03 Thread Dmitry Torokhov
On Tue, Jan 03, 2017 at 10:29:34PM +0100, Benjamin Tissoires wrote:
> On Jan 03 2017 or thereabouts, Dmitry Torokhov wrote:
>
> > Yet another option: can we add a new flag to i2c_board_info controlling
> > whether we want to enable/disable wiring up host notify interrupt?
> 
> That should be fairly easy to implement. For now, given that only Elan
> and Synaptics are the one in need for Host Notify, it could be better to
> request the Host Notify IRQ instead of disabling it unconditionally
> (which would make the current yet 8 years old lis3lv02d driver happy
> again).

I like that we have it done in i2c core instead of having drivers
implementing it individually. Since you are saying that handling host
notify is property of the slave/driver maybe we should be adding a flag
to the *i2c_driver* structure to let i2c core that we want to have host
notify mapped as interrupt if "native" interrupt is not supplied by the
platform code?

Thanks.

-- 
Dmitry


Re: [PATCH 2/2] Fix warning during compilation

2017-01-03 Thread Ivan Stoyanov
I have done this patch for mainline because since v4.3 the ipu driver 
doesn't work properly.
The reason is mistake in v4.3 considering both functions ipu_irq_err and 
ipu_irq_fn for equal and they was merged to ipu_irq_handler.

But there is only one difference between these functions which was missed.

In ipu_irq_err:
for (i = IPU_IRQ_NR_FN_BANKS; i < IPU_IRQ_NR_BANKS; i++) {

In ipu_irq_fn:
for (i = 0; i < IPU_IRQ_NR_FN_BANKS; i++) {

All kernel versions since v4.3 make kernel to freeze on boot.

Kind regards,
Ivan

On 3.1.2017 г. 23:57 ч., Arnd Bergmann wrote:

On Tuesday, January 3, 2017 10:23:29 AM CET ivan.stoya...@amk-drives.bg wrote:

From: amk 

drivers/dma/ipu/ipu_irq.c: In function 'ipu_irq_fn':
drivers/dma/ipu/ipu_irq.c:342:4: warning: 'irq' may be used uninitialized in 
this function [-Wmaybe-uninitialized]

Signed-off-by: amk 
---

This looks like my patch 86c7e6836479 ("dmaengine: ipu: remove bogus NO_IRQ 
reference")
that was applied in September, but it seems to be written for an older kernel
prior to v4.3.

Which kernel version were you testing on?

Arnd


--
Best regards,

Ivan Stoyanov

AMK "Drives and Controls" Ltd.
Bulgaria
5300 Gabrovo
1 "Gen. Nikolov" str.
www.amk-drives.bg

Phone:  +359 / 66 / 819136
E-mail: ivan.stoya...@amk-drives.bg



Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on Dell machines

2017-01-03 Thread Dmitry Torokhov
On Tue, Jan 03, 2017 at 10:29:34PM +0100, Benjamin Tissoires wrote:
> On Jan 03 2017 or thereabouts, Dmitry Torokhov wrote:
>
> > Yet another option: can we add a new flag to i2c_board_info controlling
> > whether we want to enable/disable wiring up host notify interrupt?
> 
> That should be fairly easy to implement. For now, given that only Elan
> and Synaptics are the one in need for Host Notify, it could be better to
> request the Host Notify IRQ instead of disabling it unconditionally
> (which would make the current yet 8 years old lis3lv02d driver happy
> again).

I like that we have it done in i2c core instead of having drivers
implementing it individually. Since you are saying that handling host
notify is property of the slave/driver maybe we should be adding a flag
to the *i2c_driver* structure to let i2c core that we want to have host
notify mapped as interrupt if "native" interrupt is not supplied by the
platform code?

Thanks.

-- 
Dmitry


Re: [PATCH 2/2] Fix warning during compilation

2017-01-03 Thread Ivan Stoyanov
I have done this patch for mainline because since v4.3 the ipu driver 
doesn't work properly.
The reason is mistake in v4.3 considering both functions ipu_irq_err and 
ipu_irq_fn for equal and they was merged to ipu_irq_handler.

But there is only one difference between these functions which was missed.

In ipu_irq_err:
for (i = IPU_IRQ_NR_FN_BANKS; i < IPU_IRQ_NR_BANKS; i++) {

In ipu_irq_fn:
for (i = 0; i < IPU_IRQ_NR_FN_BANKS; i++) {

All kernel versions since v4.3 make kernel to freeze on boot.

Kind regards,
Ivan

On 3.1.2017 г. 23:57 ч., Arnd Bergmann wrote:

On Tuesday, January 3, 2017 10:23:29 AM CET ivan.stoya...@amk-drives.bg wrote:

From: amk 

drivers/dma/ipu/ipu_irq.c: In function 'ipu_irq_fn':
drivers/dma/ipu/ipu_irq.c:342:4: warning: 'irq' may be used uninitialized in 
this function [-Wmaybe-uninitialized]

Signed-off-by: amk 
---

This looks like my patch 86c7e6836479 ("dmaengine: ipu: remove bogus NO_IRQ 
reference")
that was applied in September, but it seems to be written for an older kernel
prior to v4.3.

Which kernel version were you testing on?

Arnd


--
Best regards,

Ivan Stoyanov

AMK "Drives and Controls" Ltd.
Bulgaria
5300 Gabrovo
1 "Gen. Nikolov" str.
www.amk-drives.bg

Phone:  +359 / 66 / 819136
E-mail: ivan.stoya...@amk-drives.bg



Re: [PATCH v5 0/2] change the proc handler for nsm_use_hostnames

2017-01-03 Thread hejianet

Ping, any comments are welcome. Thanks

B.R.

Jia


On 15/12/2016 3:24 PM, Jia He wrote:

nsm_use_hostnames is a module parameter and it will be exported to sysctl
procfs. This is to let user sometimes change it from userspace. But the
minimal unit for sysctl procfs read/write it sizeof(int).
In big endian system, the converting from/to  bool to/from int will cause
error for proc items.

This patch introduces a new proc handler proc_dobool for nsm_use_hostnames.

Changes:
v5: Fix compilation error when CONFIG_PROC_SYSCTL is not set
v4: Change (u8 *) to (bool *)
v3: Introduce a new proc handler proc_dou8vec(suggested by Xinhui Pan)
v2: Change extern type in lockd.h

The test case I used:
/***/
#include 
#include 
#include 

bool __read_mostly nsm_use_hostnames;
module_param(nsm_use_hostnames, bool, 0644);

static struct ctl_table my_sysctl[] = {
 {
 .procname   = "nsm_use_hostnames",
 .data   = _use_hostnames,
 .maxlen = sizeof(int),
 .mode   = 0644,
 .proc_handler   = _dointvec,
 },
 {}
};

static struct ctl_table my_root[] = {
 {
 .procname   = "mysysctl",
 .mode   = 0555,
 .child  = my_sysctl,
 },
 {}
};

static struct ctl_table_header * my_ctl_header;

static int __init sysctl_exam_init(void)
{
 my_ctl_header = register_sysctl_table(_root);
 if (my_ctl_header == NULL)
 printk("error regiester sysctl");

 return 0;
}

static void __exit sysctl_exam_exit(void)
{
 unregister_sysctl_table(my_ctl_header);
}

module_init(sysctl_exam_init);
module_exit(sysctl_exam_exit);
MODULE_LICENSE("GPL");
//

[root@bigendian my]# insmod -f /root/my/hello.ko nsm_use_hostnames=1
[root@bigendian my]# cat /proc/sys/mysysctl/nsm_use_hostnames
16777216

After I change the proc_dointvec to new handler proc_dou8vec with the
patch:
[root@bigendian my]# insmod -f /root/my/hello.ko nsm_use_hostnames=1
[root@bigendian my]# cat /proc/sys/mysysctl/nsm_use_hostnames
1

In little endian system, there is no such issue.
Already tested in both of big and little endian(ppc64 and ppc64le)

Jia He (2):
   sysctl: introduce new proc handler proc_dobool
   lockd: change the proc_handler for nsm_use_hostnames

  fs/lockd/svc.c |  2 +-
  include/linux/sysctl.h |  2 ++
  kernel/sysctl.c| 41 +
  3 files changed, 44 insertions(+), 1 deletion(-)




Re: [PATCH v5 0/2] change the proc handler for nsm_use_hostnames

2017-01-03 Thread hejianet

Ping, any comments are welcome. Thanks

B.R.

Jia


On 15/12/2016 3:24 PM, Jia He wrote:

nsm_use_hostnames is a module parameter and it will be exported to sysctl
procfs. This is to let user sometimes change it from userspace. But the
minimal unit for sysctl procfs read/write it sizeof(int).
In big endian system, the converting from/to  bool to/from int will cause
error for proc items.

This patch introduces a new proc handler proc_dobool for nsm_use_hostnames.

Changes:
v5: Fix compilation error when CONFIG_PROC_SYSCTL is not set
v4: Change (u8 *) to (bool *)
v3: Introduce a new proc handler proc_dou8vec(suggested by Xinhui Pan)
v2: Change extern type in lockd.h

The test case I used:
/***/
#include 
#include 
#include 

bool __read_mostly nsm_use_hostnames;
module_param(nsm_use_hostnames, bool, 0644);

static struct ctl_table my_sysctl[] = {
 {
 .procname   = "nsm_use_hostnames",
 .data   = _use_hostnames,
 .maxlen = sizeof(int),
 .mode   = 0644,
 .proc_handler   = _dointvec,
 },
 {}
};

static struct ctl_table my_root[] = {
 {
 .procname   = "mysysctl",
 .mode   = 0555,
 .child  = my_sysctl,
 },
 {}
};

static struct ctl_table_header * my_ctl_header;

static int __init sysctl_exam_init(void)
{
 my_ctl_header = register_sysctl_table(_root);
 if (my_ctl_header == NULL)
 printk("error regiester sysctl");

 return 0;
}

static void __exit sysctl_exam_exit(void)
{
 unregister_sysctl_table(my_ctl_header);
}

module_init(sysctl_exam_init);
module_exit(sysctl_exam_exit);
MODULE_LICENSE("GPL");
//

[root@bigendian my]# insmod -f /root/my/hello.ko nsm_use_hostnames=1
[root@bigendian my]# cat /proc/sys/mysysctl/nsm_use_hostnames
16777216

After I change the proc_dointvec to new handler proc_dou8vec with the
patch:
[root@bigendian my]# insmod -f /root/my/hello.ko nsm_use_hostnames=1
[root@bigendian my]# cat /proc/sys/mysysctl/nsm_use_hostnames
1

In little endian system, there is no such issue.
Already tested in both of big and little endian(ppc64 and ppc64le)

Jia He (2):
   sysctl: introduce new proc handler proc_dobool
   lockd: change the proc_handler for nsm_use_hostnames

  fs/lockd/svc.c |  2 +-
  include/linux/sysctl.h |  2 ++
  kernel/sysctl.c| 41 +
  3 files changed, 44 insertions(+), 1 deletion(-)




Re: [PATCH v3] IB/umem: Release pid in error and ODP flow

2017-01-03 Thread Kenneth Lee
On Tue, Jan 03, 2017 at 12:12:24PM +0200, Leon Romanovsky wrote:
> Date: Tue, 3 Jan 2017 12:12:24 +0200
> From: Leon Romanovsky 
> To: Kenneth Lee 
> CC: dledf...@redhat.com, sean.he...@intel.com, hal.rosenst...@gmail.com,
>  robin.mur...@arm.com, jroe...@suse.de, egtv...@samfundet.no,
>  vgu...@synopsys.com, dave.han...@linux.intel.com, lstoa...@gmail.com,
>  k...@kernel.org, seb...@linux.vnet.ibm.com, ma...@mellanox.com,
>  linux-r...@vger.kernel.org, linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v3] IB/umem: Release pid in error and ODP flow
> User-Agent: Mutt/1.7.2 (2016-11-26)
> Message-ID: <20170103101224.GH12077@mtr-leonro.local>
> 
> On Tue, Jan 03, 2017 at 10:32:50AM +0800, Kenneth Lee wrote:
> > On Sun, Jan 01, 2017 at 08:47:12AM +0200, Leon Romanovsky wrote:
> > > Date: Sun, 1 Jan 2017 08:47:12 +0200
> > > From: Leon Romanovsky 
> > > To: Kenneth Lee 
> > > CC: dledf...@redhat.com, sean.he...@intel.com, hal.rosenst...@gmail.com,
> > >  robin.mur...@arm.com, jroe...@suse.de, egtv...@samfundet.no,
> > >  vgu...@synopsys.com, dave.han...@linux.intel.com, lstoa...@gmail.com,
> > >  k...@kernel.org, seb...@linux.vnet.ibm.com, ma...@mellanox.com,
> > >  linux-r...@vger.kernel.org, linux-kernel@vger.kernel.org
> > > Subject: Re: [PATCH v3] IB/umem: Release pid in error and ODP flow
> > > User-Agent: Mutt/1.7.2 (2016-11-26)
> > > Message-ID: <20170101064712.GQ26885@mtr-leonro.local>
> > >
> > > On Fri, Dec 30, 2016 at 06:18:29PM +0800, Kenneth Lee wrote:
> > > > There are two bugfixes in this patch:
> > > >
> > > > Fixes: 87773dd56d54 ("IB: ib_umem_release() should decrement 
> > > > mm->pinned_vm from ib_umem_get")
> > > > This patch introduce the get_task_pid but not put it back on 
> > > > all error
> > > > path
> > > >
> > > > Fixes: 8ada2c1c0c1d ("IB/core: Add support for on demand paging 
> > > > regions")
> > > > This patch introduce a ODP flow without release pid before 
> > > > enter it
> > > >
> > > >
> > > > Signed-off-by: Kenneth Lee 
> > > > Reviewed-by: Haggai Eran 
> > > > ---
> > > > Change from v1 to v2:
> > > >   Correcting the patch title and description
> > > > Change from v2 to v3:
> > > >   Update the title and add "Fixes" fields in the description
> > >
> > > OK,
> > >
> > > I see that you still didn't read Documentation/SubmittingPatches. You
> > > must read that document before you are sending patches.
> > >
> > > But I'll stop here, the code is correct (it fixes bugs) and commit message
> > > more usefull than before.
> > >
> > >
> > > >
> > > >  drivers/infiniband/core/umem.c | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/drivers/infiniband/core/umem.c 
> > > > b/drivers/infiniband/core/umem.c
> > > > index 1e62a5f..4609b92 100644
> > > > --- a/drivers/infiniband/core/umem.c
> > > > +++ b/drivers/infiniband/core/umem.c
> > > > @@ -134,6 +134,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext 
> > > > *context, unsigned long addr,
> > > >  IB_ACCESS_REMOTE_ATOMIC | IB_ACCESS_MW_BIND));
> > > >
> > > > if (access & IB_ACCESS_ON_DEMAND) {
> > > > +   put_pid(umem->pid);
> > > > ret = ib_umem_odp_get(context, umem);
> > > > if (ret) {
> > > > kfree(umem);
> > > > @@ -149,6 +150,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext 
> > > > *context, unsigned long addr,
> > > >
> > > > page_list = (struct page **) __get_free_page(GFP_KERNEL);
> > > > if (!page_list) {
> > > > +   put_pid(umem->pid);
> > > > kfree(umem);
> > > > return ERR_PTR(-ENOMEM);
> > > > }
> > > > --
> > > > 1.9.1
> > > >
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> > > > the body of a message to majord...@vger.kernel.org
> > > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> > Thanks,
> >
> > I did read the doc, but maybe I mis-understant some points. Could you please
> > point it out?
> 
> Fixes line should be placed above bottom signatures.
> 
> As an example of properly written patch, you can take a look on the
> following patch [1] from Steve.
> 
> [1] http://marc.info/?l=linux-rdma=148244272205411=2

Thank you. A sample help a lot.

But please allow me to argue a little:
Documentation/process/submitting-patches.rst does really not mention where Fixes
tags should be put:)

> 
> >
> > And sorry. please ignore the last message. I forget to use a bottom-post 
> > style.
> >
> >
> >
> > --
> > -Kenneth(Hisilicon)



-- 
-Kenneth(Hisilicon)


Re: Designated initializers, struct randomization and addressing?

2017-01-03 Thread Kees Cook
On Tue, Jan 3, 2017 at 10:27 PM, Julia Lawall  wrote:
>
>
> On Tue, 3 Jan 2017, Kees Cook wrote:
>
>> On Tue, Dec 20, 2016 at 9:29 AM, Joe Perches  wrote:
>> > On Fri, 2016-12-16 at 17:00 -0800, Kees Cook wrote:
>> >> Prepare to mark sensitive kernel structures for randomization by making
>> > sure they're using designated initializers.
>> >
>> > About the designated initializer patches,
>> > which by themselves are fine of course,
>> > and the fundamental randomization plugin,
>> > c guarantees that struct member ordering
>> > is as specified.
>> >
>> > how is the code to be verified so that
>> > any use of things like offsetof and any
>> > address/indexing is not impacted?
>>
>> AIUI, offsetof() works correctly in the face of this plugin, since the
>> ordering happens before the pass that handles offsetof(). Anything
>> that _does not_ use offsetof(), however, needs fixing. Based on the
>> work done in grsecurity, I don't see any added offsetof() uses that
>> are specific to the randomization plugin.
>>
>> (Note that the randomization plugin is only on function pointer
>> structures, where using an offsetof() should be rare to none, and on
>> hand-selected structures, where missing offsetof() should be easy to
>> audit.)
>
> What is the precise definition of "function pointer structures"?  Only
> function pointers?  At least one function pointer?

For randstruct and constify, the automatic selection is done on
structures with only function pointers. (Additional structures can be
added via a compiler attribute marking.)

See is_pure_ops_struct():

http://git.kernel.org/cgit/linux/kernel/git/kees/linux.git/tree/scripts/gcc-plugins/randomize_layout_plugin.c?h=kspp/gcc-plugin/randstruct

-Kees

-- 
Kees Cook
Nexus Security


Re: [PATCH v3] IB/umem: Release pid in error and ODP flow

2017-01-03 Thread Kenneth Lee
On Tue, Jan 03, 2017 at 12:12:24PM +0200, Leon Romanovsky wrote:
> Date: Tue, 3 Jan 2017 12:12:24 +0200
> From: Leon Romanovsky 
> To: Kenneth Lee 
> CC: dledf...@redhat.com, sean.he...@intel.com, hal.rosenst...@gmail.com,
>  robin.mur...@arm.com, jroe...@suse.de, egtv...@samfundet.no,
>  vgu...@synopsys.com, dave.han...@linux.intel.com, lstoa...@gmail.com,
>  k...@kernel.org, seb...@linux.vnet.ibm.com, ma...@mellanox.com,
>  linux-r...@vger.kernel.org, linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v3] IB/umem: Release pid in error and ODP flow
> User-Agent: Mutt/1.7.2 (2016-11-26)
> Message-ID: <20170103101224.GH12077@mtr-leonro.local>
> 
> On Tue, Jan 03, 2017 at 10:32:50AM +0800, Kenneth Lee wrote:
> > On Sun, Jan 01, 2017 at 08:47:12AM +0200, Leon Romanovsky wrote:
> > > Date: Sun, 1 Jan 2017 08:47:12 +0200
> > > From: Leon Romanovsky 
> > > To: Kenneth Lee 
> > > CC: dledf...@redhat.com, sean.he...@intel.com, hal.rosenst...@gmail.com,
> > >  robin.mur...@arm.com, jroe...@suse.de, egtv...@samfundet.no,
> > >  vgu...@synopsys.com, dave.han...@linux.intel.com, lstoa...@gmail.com,
> > >  k...@kernel.org, seb...@linux.vnet.ibm.com, ma...@mellanox.com,
> > >  linux-r...@vger.kernel.org, linux-kernel@vger.kernel.org
> > > Subject: Re: [PATCH v3] IB/umem: Release pid in error and ODP flow
> > > User-Agent: Mutt/1.7.2 (2016-11-26)
> > > Message-ID: <20170101064712.GQ26885@mtr-leonro.local>
> > >
> > > On Fri, Dec 30, 2016 at 06:18:29PM +0800, Kenneth Lee wrote:
> > > > There are two bugfixes in this patch:
> > > >
> > > > Fixes: 87773dd56d54 ("IB: ib_umem_release() should decrement 
> > > > mm->pinned_vm from ib_umem_get")
> > > > This patch introduce the get_task_pid but not put it back on 
> > > > all error
> > > > path
> > > >
> > > > Fixes: 8ada2c1c0c1d ("IB/core: Add support for on demand paging 
> > > > regions")
> > > > This patch introduce a ODP flow without release pid before 
> > > > enter it
> > > >
> > > >
> > > > Signed-off-by: Kenneth Lee 
> > > > Reviewed-by: Haggai Eran 
> > > > ---
> > > > Change from v1 to v2:
> > > >   Correcting the patch title and description
> > > > Change from v2 to v3:
> > > >   Update the title and add "Fixes" fields in the description
> > >
> > > OK,
> > >
> > > I see that you still didn't read Documentation/SubmittingPatches. You
> > > must read that document before you are sending patches.
> > >
> > > But I'll stop here, the code is correct (it fixes bugs) and commit message
> > > more usefull than before.
> > >
> > >
> > > >
> > > >  drivers/infiniband/core/umem.c | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/drivers/infiniband/core/umem.c 
> > > > b/drivers/infiniband/core/umem.c
> > > > index 1e62a5f..4609b92 100644
> > > > --- a/drivers/infiniband/core/umem.c
> > > > +++ b/drivers/infiniband/core/umem.c
> > > > @@ -134,6 +134,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext 
> > > > *context, unsigned long addr,
> > > >  IB_ACCESS_REMOTE_ATOMIC | IB_ACCESS_MW_BIND));
> > > >
> > > > if (access & IB_ACCESS_ON_DEMAND) {
> > > > +   put_pid(umem->pid);
> > > > ret = ib_umem_odp_get(context, umem);
> > > > if (ret) {
> > > > kfree(umem);
> > > > @@ -149,6 +150,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext 
> > > > *context, unsigned long addr,
> > > >
> > > > page_list = (struct page **) __get_free_page(GFP_KERNEL);
> > > > if (!page_list) {
> > > > +   put_pid(umem->pid);
> > > > kfree(umem);
> > > > return ERR_PTR(-ENOMEM);
> > > > }
> > > > --
> > > > 1.9.1
> > > >
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> > > > the body of a message to majord...@vger.kernel.org
> > > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> > Thanks,
> >
> > I did read the doc, but maybe I mis-understant some points. Could you please
> > point it out?
> 
> Fixes line should be placed above bottom signatures.
> 
> As an example of properly written patch, you can take a look on the
> following patch [1] from Steve.
> 
> [1] http://marc.info/?l=linux-rdma=148244272205411=2

Thank you. A sample help a lot.

But please allow me to argue a little:
Documentation/process/submitting-patches.rst does really not mention where Fixes
tags should be put:)

> 
> >
> > And sorry. please ignore the last message. I forget to use a bottom-post 
> > style.
> >
> >
> >
> > --
> > -Kenneth(Hisilicon)



-- 
-Kenneth(Hisilicon)


Re: Designated initializers, struct randomization and addressing?

2017-01-03 Thread Kees Cook
On Tue, Jan 3, 2017 at 10:27 PM, Julia Lawall  wrote:
>
>
> On Tue, 3 Jan 2017, Kees Cook wrote:
>
>> On Tue, Dec 20, 2016 at 9:29 AM, Joe Perches  wrote:
>> > On Fri, 2016-12-16 at 17:00 -0800, Kees Cook wrote:
>> >> Prepare to mark sensitive kernel structures for randomization by making
>> > sure they're using designated initializers.
>> >
>> > About the designated initializer patches,
>> > which by themselves are fine of course,
>> > and the fundamental randomization plugin,
>> > c guarantees that struct member ordering
>> > is as specified.
>> >
>> > how is the code to be verified so that
>> > any use of things like offsetof and any
>> > address/indexing is not impacted?
>>
>> AIUI, offsetof() works correctly in the face of this plugin, since the
>> ordering happens before the pass that handles offsetof(). Anything
>> that _does not_ use offsetof(), however, needs fixing. Based on the
>> work done in grsecurity, I don't see any added offsetof() uses that
>> are specific to the randomization plugin.
>>
>> (Note that the randomization plugin is only on function pointer
>> structures, where using an offsetof() should be rare to none, and on
>> hand-selected structures, where missing offsetof() should be easy to
>> audit.)
>
> What is the precise definition of "function pointer structures"?  Only
> function pointers?  At least one function pointer?

For randstruct and constify, the automatic selection is done on
structures with only function pointers. (Additional structures can be
added via a compiler attribute marking.)

See is_pure_ops_struct():

http://git.kernel.org/cgit/linux/kernel/git/kees/linux.git/tree/scripts/gcc-plugins/randomize_layout_plugin.c?h=kspp/gcc-plugin/randstruct

-Kees

-- 
Kees Cook
Nexus Security


Re: [PATCH 0/2] Begin auditing SECCOMP_RET_ERRNO return actions

2017-01-03 Thread Kees Cook
On Tue, Jan 3, 2017 at 8:43 PM, Richard Guy Briggs  wrote:
> On 2017-01-04 08:58, Tyler Hicks wrote:
>> On 01/04/2017 04:44 AM, Kees Cook wrote:
>> > On Tue, Jan 3, 2017 at 1:31 PM, Paul Moore  wrote:
>> >> On Tue, Jan 3, 2017 at 4:21 PM, Kees Cook  wrote:
>> >>> On Tue, Jan 3, 2017 at 1:13 PM, Paul Moore  wrote:
>>  On Tue, Jan 3, 2017 at 4:03 PM, Kees Cook  wrote:
>> > On Tue, Jan 3, 2017 at 12:54 PM, Paul Moore  
>> > wrote:
>> >> On Tue, Jan 3, 2017 at 3:44 PM, Kees Cook  
>> >> wrote:
>> >>> I still wonder, though, isn't there a way to use auditctl to get all
>> >>> the seccomp messages you need?
>> >>
>> >> Not all of the seccomp actions are currently logged, that's one of the
>> >> problems (and the biggest at the moment).
>> >
>> > Well... sort of. It all gets passed around, but the logic isn't very
>> > obvious (or at least I always have to go look it up).
>> 
>>  Last time I checked SECCOMP_RET_ALLOW wasn't logged (as well as at
>>  least one other action, but I can't remember which off the top of my
>>  head)?
>> >>>
>> >>> Sure, but if you're using audit, you don't need RET_ALLOW to be logged
>> >>> because you'll get a full syscall log entry. Logging RET_ALLOW is
>> >>> redundant and provides no new information, it seems to me.
>> >>
>> >> I only bring this up as it might be a way to help solve the
>> >> SECCOMP_RET_AUDIT problem that Tyler mentioned.
>> >
>> > So, I guess I want to understand why something like this doesn't work,
>> > with no changes at all to the kernel:
>> >
>> > Imaginary "seccomp-audit.c":
>> >
>> > ...
>> > pid = fork();
>> > if (pid) {
>> > char cmd[80];
>> >
>> > sprintf(cmd, "auditctl -a always,exit -S all -F pid=%d", pid);
>> > system(cmd);
>> > release...
>> >  } else {
>> > wait for release...
>> > execv(argv[1], argv + 1);
>> >  }
>> > ...
>> >
>> > This should dump all syscalls (both RET_ALLOW and RET_ERRNO), as well
>> > as all seccomp actions of any kind. (Down side is the need for root to
>> > launch auditctl...)
>>
>> Hey Kees - Thanks for the suggestion!
>>
>> Here are some of the reasons that it doesn't quite work:
>>
>> 1) We don't install/run auditd by default and would continue to prefer
>> not to in some situations where resources are tight.

Strictly speaking, auditd isn't needed for auditctl, IIUC.

>> 2) We block a relatively small number of syscalls as compared to what
>> are allowed so auditing all syscalls is a really heavyweight solution.
>> That could be fixed with a better -S argument, though.

Yeah, it seems like there needs to be some kind of improvement there
on the audit side (I was thinking a better -F). The all-or-nothing
approach is way too big a hammer.

>> 3) We sometimes only block certain arguments for a given syscall and
>> auditing all instances of the syscall could still be a heavyweight solution.
>>
>> 4) If the application spawns children processes, that rule doesn't audit
>> their syscalls. That can be fixed with ppid=%d but then grandchildren
>> pids are a problem.
>
> This patch that wasn't accepted upstream might be useful:
> https://www.redhat.com/archives/linux-audit/2015-August/msg00067.html
> https://www.redhat.com/archives/linux-audit/2015-August/msg00068.html

I'd like this regardless. It's really difficult to audit trees of
processes before they launch. :)

>
>> 5) Cleanup of the audit rule for an old pid, before the pid is reused,
>> could be difficult.
>>
>> Tyler
>>
>> > Perhaps an improvement to this could be enabling audit when seccomp
>> > syscall is seen? I can't tell if auditctl already has something to do
>> > this ("start auditing this process and all children when syscall X is
>> > performed").
>> >
>> > -Kees
>
> - RGB
>
> --
> Richard Guy Briggs 
> Kernel Security Engineering, Base Operating Systems, Red Hat
> Remote, Ottawa, Canada
> Voice: +1.647.777.2635, Internal: (81) 32635

-Kees


-- 
Kees Cook
Nexus Security


Re: [PATCH 0/2] Begin auditing SECCOMP_RET_ERRNO return actions

2017-01-03 Thread Kees Cook
On Tue, Jan 3, 2017 at 8:43 PM, Richard Guy Briggs  wrote:
> On 2017-01-04 08:58, Tyler Hicks wrote:
>> On 01/04/2017 04:44 AM, Kees Cook wrote:
>> > On Tue, Jan 3, 2017 at 1:31 PM, Paul Moore  wrote:
>> >> On Tue, Jan 3, 2017 at 4:21 PM, Kees Cook  wrote:
>> >>> On Tue, Jan 3, 2017 at 1:13 PM, Paul Moore  wrote:
>>  On Tue, Jan 3, 2017 at 4:03 PM, Kees Cook  wrote:
>> > On Tue, Jan 3, 2017 at 12:54 PM, Paul Moore  
>> > wrote:
>> >> On Tue, Jan 3, 2017 at 3:44 PM, Kees Cook  
>> >> wrote:
>> >>> I still wonder, though, isn't there a way to use auditctl to get all
>> >>> the seccomp messages you need?
>> >>
>> >> Not all of the seccomp actions are currently logged, that's one of the
>> >> problems (and the biggest at the moment).
>> >
>> > Well... sort of. It all gets passed around, but the logic isn't very
>> > obvious (or at least I always have to go look it up).
>> 
>>  Last time I checked SECCOMP_RET_ALLOW wasn't logged (as well as at
>>  least one other action, but I can't remember which off the top of my
>>  head)?
>> >>>
>> >>> Sure, but if you're using audit, you don't need RET_ALLOW to be logged
>> >>> because you'll get a full syscall log entry. Logging RET_ALLOW is
>> >>> redundant and provides no new information, it seems to me.
>> >>
>> >> I only bring this up as it might be a way to help solve the
>> >> SECCOMP_RET_AUDIT problem that Tyler mentioned.
>> >
>> > So, I guess I want to understand why something like this doesn't work,
>> > with no changes at all to the kernel:
>> >
>> > Imaginary "seccomp-audit.c":
>> >
>> > ...
>> > pid = fork();
>> > if (pid) {
>> > char cmd[80];
>> >
>> > sprintf(cmd, "auditctl -a always,exit -S all -F pid=%d", pid);
>> > system(cmd);
>> > release...
>> >  } else {
>> > wait for release...
>> > execv(argv[1], argv + 1);
>> >  }
>> > ...
>> >
>> > This should dump all syscalls (both RET_ALLOW and RET_ERRNO), as well
>> > as all seccomp actions of any kind. (Down side is the need for root to
>> > launch auditctl...)
>>
>> Hey Kees - Thanks for the suggestion!
>>
>> Here are some of the reasons that it doesn't quite work:
>>
>> 1) We don't install/run auditd by default and would continue to prefer
>> not to in some situations where resources are tight.

Strictly speaking, auditd isn't needed for auditctl, IIUC.

>> 2) We block a relatively small number of syscalls as compared to what
>> are allowed so auditing all syscalls is a really heavyweight solution.
>> That could be fixed with a better -S argument, though.

Yeah, it seems like there needs to be some kind of improvement there
on the audit side (I was thinking a better -F). The all-or-nothing
approach is way too big a hammer.

>> 3) We sometimes only block certain arguments for a given syscall and
>> auditing all instances of the syscall could still be a heavyweight solution.
>>
>> 4) If the application spawns children processes, that rule doesn't audit
>> their syscalls. That can be fixed with ppid=%d but then grandchildren
>> pids are a problem.
>
> This patch that wasn't accepted upstream might be useful:
> https://www.redhat.com/archives/linux-audit/2015-August/msg00067.html
> https://www.redhat.com/archives/linux-audit/2015-August/msg00068.html

I'd like this regardless. It's really difficult to audit trees of
processes before they launch. :)

>
>> 5) Cleanup of the audit rule for an old pid, before the pid is reused,
>> could be difficult.
>>
>> Tyler
>>
>> > Perhaps an improvement to this could be enabling audit when seccomp
>> > syscall is seen? I can't tell if auditctl already has something to do
>> > this ("start auditing this process and all children when syscall X is
>> > performed").
>> >
>> > -Kees
>
> - RGB
>
> --
> Richard Guy Briggs 
> Kernel Security Engineering, Base Operating Systems, Red Hat
> Remote, Ottawa, Canada
> Voice: +1.647.777.2635, Internal: (81) 32635

-Kees


-- 
Kees Cook
Nexus Security


Re: [PATCH 1/2] arm64: dma_mapping: allow PCI host driver to limit DMA mask

2017-01-03 Thread Nikita Yushchenko
> commit 9a57d58d116800a535510053136c6dd7a9c26e25
> Author: Arnd Bergmann 
> Date:   Tue Nov 17 14:06:55 2015 +0100
> 
> [EXPERIMENTAL] ARM64: check implement dma_set_mask
> 
> Needs work for coherent mask
> 
> Signed-off-by: Arnd Bergmann 

Unfortunately this is far incomplete

> @@ -957,6 +983,18 @@ void arch_setup_dma_ops(struct device *dev, u64 
> dma_base, u64 size,
>   if (!dev->archdata.dma_ops)
>   dev->archdata.dma_ops = _dma_ops;
>  
> + /*
> +  * we don't yet support buses that have a non-zero mapping.
> +  *  Let's hope we won't need it
> +  */
> + WARN_ON(dma_base != 0);
> +
> + /*
> +  * Whatever the parent bus can set. A device must not set
> +  * a DMA mask larger than this.
> +  */
> + dev->archdata.parent_dma_mask = size;
> +

... because size/mask passed here for PCI devices are meaningless.

For OF platforms, this is called via of_dma_configure(), that checks
dma-ranges of node that is *parent* for host bridge. Host bridge
currently does not control this at all.

In current device trees no dma-ranges is defined for nodes that are
parents to pci host bridges. This will make of_dma_configure() to fall
back to 32-bit size for all devices on all current platforms.  Thus
applying this patch will immediately break 64-bit dma masks on all
hardware that supports it.


Also related: dma-ranges property used by several pci host bridges is
*not* compatible with "legacy" dma-ranges parsed by of_get_dma_range() -
former uses additional flags word at beginning.


Re: [PATCH 1/2] arm64: dma_mapping: allow PCI host driver to limit DMA mask

2017-01-03 Thread Nikita Yushchenko
> commit 9a57d58d116800a535510053136c6dd7a9c26e25
> Author: Arnd Bergmann 
> Date:   Tue Nov 17 14:06:55 2015 +0100
> 
> [EXPERIMENTAL] ARM64: check implement dma_set_mask
> 
> Needs work for coherent mask
> 
> Signed-off-by: Arnd Bergmann 

Unfortunately this is far incomplete

> @@ -957,6 +983,18 @@ void arch_setup_dma_ops(struct device *dev, u64 
> dma_base, u64 size,
>   if (!dev->archdata.dma_ops)
>   dev->archdata.dma_ops = _dma_ops;
>  
> + /*
> +  * we don't yet support buses that have a non-zero mapping.
> +  *  Let's hope we won't need it
> +  */
> + WARN_ON(dma_base != 0);
> +
> + /*
> +  * Whatever the parent bus can set. A device must not set
> +  * a DMA mask larger than this.
> +  */
> + dev->archdata.parent_dma_mask = size;
> +

... because size/mask passed here for PCI devices are meaningless.

For OF platforms, this is called via of_dma_configure(), that checks
dma-ranges of node that is *parent* for host bridge. Host bridge
currently does not control this at all.

In current device trees no dma-ranges is defined for nodes that are
parents to pci host bridges. This will make of_dma_configure() to fall
back to 32-bit size for all devices on all current platforms.  Thus
applying this patch will immediately break 64-bit dma masks on all
hardware that supports it.


Also related: dma-ranges property used by several pci host bridges is
*not* compatible with "legacy" dma-ranges parsed by of_get_dma_range() -
former uses additional flags word at beginning.


[PATCH] ACPI/PCI: Fix bus range comparation in pci_mcfg_lookup

2017-01-03 Thread Zhou Wang
The configuration data provided by an MCFG region (ie PCI segment and
bus range) may span multiple host bridges.

Current code in pci_mcfg_lookup() carries out an exact match of host
bridge bus range start value against the MCFG region(s) bus range start
value which would cause configurations like the following:

MCFG region:
bus range: 0x00~0xff.
segment: 0.

PCI host bridges configuration (segment numbers and bus ranges):
host bridge 1:
bus range: 0x00~0x1f.
segment: 0.
host bridge 2:
bus range: 0x20~0x4f.
segment: 0.

to fail, in that the bus range start value for host bridge 2 does
not match the bus range start value of the respective MCFG region.

Relax the bus range check in pci_mcfg_lookup() to cater for
PCI configurations with multiple host bridges sharing the same
MCFG region.

Signed-off-by: Zhou Wang 
Reviewed-by: Tomasz Nowicki 
Acked-by: Lorenzo Pieralisi 
---
 drivers/acpi/pci_mcfg.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
index a6a4cea..2944353 100644
--- a/drivers/acpi/pci_mcfg.c
+++ b/drivers/acpi/pci_mcfg.c
@@ -195,11 +195,10 @@ int pci_mcfg_lookup(struct acpi_pci_root *root, struct 
resource *cfgres,
goto skip_lookup;
 
/*
-* We expect exact match, unless MCFG entry end bus covers more than
-* specified by caller.
+* We expect the range in bus_res in the coverage of MCFG bus range.
 */
list_for_each_entry(e, _mcfg_list, list) {
-   if (e->segment == seg && e->bus_start == bus_res->start &&
+   if (e->segment == seg && e->bus_start <= bus_res->start &&
e->bus_end >= bus_res->end) {
root->mcfg_addr = e->addr;
}
-- 
1.9.1



[PATCH] ACPI/PCI: Fix bus range comparation in pci_mcfg_lookup

2017-01-03 Thread Zhou Wang
The configuration data provided by an MCFG region (ie PCI segment and
bus range) may span multiple host bridges.

Current code in pci_mcfg_lookup() carries out an exact match of host
bridge bus range start value against the MCFG region(s) bus range start
value which would cause configurations like the following:

MCFG region:
bus range: 0x00~0xff.
segment: 0.

PCI host bridges configuration (segment numbers and bus ranges):
host bridge 1:
bus range: 0x00~0x1f.
segment: 0.
host bridge 2:
bus range: 0x20~0x4f.
segment: 0.

to fail, in that the bus range start value for host bridge 2 does
not match the bus range start value of the respective MCFG region.

Relax the bus range check in pci_mcfg_lookup() to cater for
PCI configurations with multiple host bridges sharing the same
MCFG region.

Signed-off-by: Zhou Wang 
Reviewed-by: Tomasz Nowicki 
Acked-by: Lorenzo Pieralisi 
---
 drivers/acpi/pci_mcfg.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
index a6a4cea..2944353 100644
--- a/drivers/acpi/pci_mcfg.c
+++ b/drivers/acpi/pci_mcfg.c
@@ -195,11 +195,10 @@ int pci_mcfg_lookup(struct acpi_pci_root *root, struct 
resource *cfgres,
goto skip_lookup;
 
/*
-* We expect exact match, unless MCFG entry end bus covers more than
-* specified by caller.
+* We expect the range in bus_res in the coverage of MCFG bus range.
 */
list_for_each_entry(e, _mcfg_list, list) {
-   if (e->segment == seg && e->bus_start == bus_res->start &&
+   if (e->segment == seg && e->bus_start <= bus_res->start &&
e->bus_end >= bus_res->end) {
root->mcfg_addr = e->addr;
}
-- 
1.9.1



Re: [PATCH v2 1/2] ALSA: usb-audio: Fix irq/process data synchronization

2017-01-03 Thread Takashi Iwai
On Mon, 02 Jan 2017 16:50:30 +0100,
Ioan-Adrian Ratiu wrote:
> 
> --- a/sound/usb/pcm.c
> +++ b/sound/usb/pcm.c
(snip)
> @@ -850,7 +850,7 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream 
> *substream)
>   /* for playback, submit the URBs now; otherwise, the first hwptr_done
>* updates for all URBs would happen at the same time when starting */
>   if (subs->direction == SNDRV_PCM_STREAM_PLAYBACK)
> - ret = start_endpoints(subs, true);
> + return start_endpoints(subs);

Here you miss the unlock below.

>  
>   unlock:
>   snd_usb_unlock_shutdown(subs->stream->chip);

... and this must be the reason of the hang up at disconnection, where
the driver ways forever at wait_event() in usb_audio_disconnect().

Could you fix this and resubmit v3?  Other than that, it looks OK.


thanks,

Takashi


Re: [PATCH v2 1/2] ALSA: usb-audio: Fix irq/process data synchronization

2017-01-03 Thread Takashi Iwai
On Mon, 02 Jan 2017 16:50:30 +0100,
Ioan-Adrian Ratiu wrote:
> 
> --- a/sound/usb/pcm.c
> +++ b/sound/usb/pcm.c
(snip)
> @@ -850,7 +850,7 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream 
> *substream)
>   /* for playback, submit the URBs now; otherwise, the first hwptr_done
>* updates for all URBs would happen at the same time when starting */
>   if (subs->direction == SNDRV_PCM_STREAM_PLAYBACK)
> - ret = start_endpoints(subs, true);
> + return start_endpoints(subs);

Here you miss the unlock below.

>  
>   unlock:
>   snd_usb_unlock_shutdown(subs->stream->chip);

... and this must be the reason of the hang up at disconnection, where
the driver ways forever at wait_event() in usb_audio_disconnect().

Could you fix this and resubmit v3?  Other than that, it looks OK.


thanks,

Takashi


Re: Designated initializers, struct randomization and addressing?

2017-01-03 Thread Julia Lawall


On Tue, 3 Jan 2017, Kees Cook wrote:

> On Tue, Dec 20, 2016 at 9:29 AM, Joe Perches  wrote:
> > On Fri, 2016-12-16 at 17:00 -0800, Kees Cook wrote:
> >> Prepare to mark sensitive kernel structures for randomization by making
> > sure they're using designated initializers.
> >
> > About the designated initializer patches,
> > which by themselves are fine of course,
> > and the fundamental randomization plugin,
> > c guarantees that struct member ordering
> > is as specified.
> >
> > how is the code to be verified so that
> > any use of things like offsetof and any
> > address/indexing is not impacted?
>
> AIUI, offsetof() works correctly in the face of this plugin, since the
> ordering happens before the pass that handles offsetof(). Anything
> that _does not_ use offsetof(), however, needs fixing. Based on the
> work done in grsecurity, I don't see any added offsetof() uses that
> are specific to the randomization plugin.
>
> (Note that the randomization plugin is only on function pointer
> structures, where using an offsetof() should be rare to none, and on
> hand-selected structures, where missing offsetof() should be easy to
> audit.)

What is the precise definition of "function pointer structures"?  Only
function pointers?  At least one function pointer?

thanks,
julia


Re: Designated initializers, struct randomization and addressing?

2017-01-03 Thread Julia Lawall


On Tue, 3 Jan 2017, Kees Cook wrote:

> On Tue, Dec 20, 2016 at 9:29 AM, Joe Perches  wrote:
> > On Fri, 2016-12-16 at 17:00 -0800, Kees Cook wrote:
> >> Prepare to mark sensitive kernel structures for randomization by making
> > sure they're using designated initializers.
> >
> > About the designated initializer patches,
> > which by themselves are fine of course,
> > and the fundamental randomization plugin,
> > c guarantees that struct member ordering
> > is as specified.
> >
> > how is the code to be verified so that
> > any use of things like offsetof and any
> > address/indexing is not impacted?
>
> AIUI, offsetof() works correctly in the face of this plugin, since the
> ordering happens before the pass that handles offsetof(). Anything
> that _does not_ use offsetof(), however, needs fixing. Based on the
> work done in grsecurity, I don't see any added offsetof() uses that
> are specific to the randomization plugin.
>
> (Note that the randomization plugin is only on function pointer
> structures, where using an offsetof() should be rare to none, and on
> hand-selected structures, where missing offsetof() should be easy to
> audit.)

What is the precise definition of "function pointer structures"?  Only
function pointers?  At least one function pointer?

thanks,
julia


[PATCH zram] extend zero pages to same element pages

2017-01-03 Thread zhouxianrong
From: z00281421 


Signed-off-by: z00281421 
---
 drivers/block/zram/zram_drv.c |   67 ++---
 drivers/block/zram/zram_drv.h |   11 ---
 2 files changed, 49 insertions(+), 29 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 15f58ab..c3af69a 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -94,6 +94,17 @@ static void zram_clear_flag(struct zram_meta *meta, u32 
index,
meta->table[index].value &= ~BIT(flag);
 }
 
+static inline void zram_set_element(struct zram_meta *meta, u32 index,
+   unsigned long element)
+{
+   meta->table[index].element = element;
+}
+
+static inline void zram_clear_element(struct zram_meta *meta, u32 index)
+{
+   meta->table[index].element = 0;
+}
+
 static size_t zram_get_obj_size(struct zram_meta *meta, u32 index)
 {
return meta->table[index].value & (BIT(ZRAM_FLAG_SHIFT) - 1);
@@ -158,31 +169,31 @@ static inline void update_used_max(struct zram *zram,
} while (old_max != cur_max);
 }
 
-static bool page_zero_filled(void *ptr)
+static bool page_same_filled(void *ptr)
 {
unsigned int pos;
unsigned long *page;
 
page = (unsigned long *)ptr;
 
-   for (pos = 0; pos != PAGE_SIZE / sizeof(*page); pos++) {
-   if (page[pos])
+   for (pos = PAGE_SIZE / sizeof(unsigned long) - 1; pos > 0; pos--) {
+   if (page[pos] != page[pos - 1])
return false;
}
 
return true;
 }
 
-static void handle_zero_page(struct bio_vec *bvec)
+static void handle_same_page(struct bio_vec *bvec, unsigned long element)
 {
struct page *page = bvec->bv_page;
void *user_mem;
 
user_mem = kmap_atomic(page);
if (is_partial_io(bvec))
-   memset(user_mem + bvec->bv_offset, 0, bvec->bv_len);
+   memset(user_mem + bvec->bv_offset, (char)element, bvec->bv_len);
else
-   clear_page(user_mem);
+   memset(user_mem, (char)element, PAGE_SIZE);
kunmap_atomic(user_mem);
 
flush_dcache_page(page);
@@ -431,7 +442,7 @@ static ssize_t mm_stat_show(struct device *dev,
mem_used << PAGE_SHIFT,
zram->limit_pages << PAGE_SHIFT,
max_used << PAGE_SHIFT,
-   (u64)atomic64_read(>stats.zero_pages),
+   (u64)atomic64_read(>stats.same_pages),
pool_stats.pages_compacted);
up_read(>init_lock);
 
@@ -464,7 +475,7 @@ static ssize_t debug_stat_show(struct device *dev,
 ZRAM_ATTR_RO(failed_writes);
 ZRAM_ATTR_RO(invalid_io);
 ZRAM_ATTR_RO(notify_free);
-ZRAM_ATTR_RO(zero_pages);
+ZRAM_ATTR_RO(same_pages);
 ZRAM_ATTR_RO(compr_data_size);
 
 static inline bool zram_meta_get(struct zram *zram)
@@ -538,18 +549,20 @@ static void zram_free_page(struct zram *zram, size_t 
index)
struct zram_meta *meta = zram->meta;
unsigned long handle = meta->table[index].handle;
 
-   if (unlikely(!handle)) {
-   /*
-* No memory is allocated for zero filled pages.
-* Simply clear zero page flag.
-*/
-   if (zram_test_flag(meta, index, ZRAM_ZERO)) {
-   zram_clear_flag(meta, index, ZRAM_ZERO);
-   atomic64_dec(>stats.zero_pages);
-   }
+   /*
+* No memory is allocated for same element filled pages.
+* Simply clear same page flag.
+*/
+   if (zram_test_flag(meta, index, ZRAM_SAME)) {
+   zram_clear_flag(meta, index, ZRAM_SAME);
+   zram_clear_element(meta, index);
+   atomic64_dec(>stats.same_pages);
return;
}
 
+   if (!handle)
+   return;
+
zs_free(meta->mem_pool, handle);
 
atomic64_sub(zram_get_obj_size(meta, index),
@@ -572,9 +585,9 @@ static int zram_decompress_page(struct zram *zram, char 
*mem, u32 index)
handle = meta->table[index].handle;
size = zram_get_obj_size(meta, index);
 
-   if (!handle || zram_test_flag(meta, index, ZRAM_ZERO)) {
+   if (!handle || zram_test_flag(meta, index, ZRAM_SAME)) {
bit_spin_unlock(ZRAM_ACCESS, >table[index].value);
-   clear_page(mem);
+   memset(mem, (char)meta->table[index].element, PAGE_SIZE);
return 0;
}
 
@@ -610,9 +623,9 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec 
*bvec,
 
bit_spin_lock(ZRAM_ACCESS, >table[index].value);
if (unlikely(!meta->table[index].handle) ||
-   zram_test_flag(meta, index, ZRAM_ZERO)) {
+   zram_test_flag(meta, index, ZRAM_SAME)) {
bit_spin_unlock(ZRAM_ACCESS, >table[index].value);
-   

[PATCH zram] extend zero pages to same element pages

2017-01-03 Thread zhouxianrong
From: z00281421 


Signed-off-by: z00281421 
---
 drivers/block/zram/zram_drv.c |   67 ++---
 drivers/block/zram/zram_drv.h |   11 ---
 2 files changed, 49 insertions(+), 29 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 15f58ab..c3af69a 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -94,6 +94,17 @@ static void zram_clear_flag(struct zram_meta *meta, u32 
index,
meta->table[index].value &= ~BIT(flag);
 }
 
+static inline void zram_set_element(struct zram_meta *meta, u32 index,
+   unsigned long element)
+{
+   meta->table[index].element = element;
+}
+
+static inline void zram_clear_element(struct zram_meta *meta, u32 index)
+{
+   meta->table[index].element = 0;
+}
+
 static size_t zram_get_obj_size(struct zram_meta *meta, u32 index)
 {
return meta->table[index].value & (BIT(ZRAM_FLAG_SHIFT) - 1);
@@ -158,31 +169,31 @@ static inline void update_used_max(struct zram *zram,
} while (old_max != cur_max);
 }
 
-static bool page_zero_filled(void *ptr)
+static bool page_same_filled(void *ptr)
 {
unsigned int pos;
unsigned long *page;
 
page = (unsigned long *)ptr;
 
-   for (pos = 0; pos != PAGE_SIZE / sizeof(*page); pos++) {
-   if (page[pos])
+   for (pos = PAGE_SIZE / sizeof(unsigned long) - 1; pos > 0; pos--) {
+   if (page[pos] != page[pos - 1])
return false;
}
 
return true;
 }
 
-static void handle_zero_page(struct bio_vec *bvec)
+static void handle_same_page(struct bio_vec *bvec, unsigned long element)
 {
struct page *page = bvec->bv_page;
void *user_mem;
 
user_mem = kmap_atomic(page);
if (is_partial_io(bvec))
-   memset(user_mem + bvec->bv_offset, 0, bvec->bv_len);
+   memset(user_mem + bvec->bv_offset, (char)element, bvec->bv_len);
else
-   clear_page(user_mem);
+   memset(user_mem, (char)element, PAGE_SIZE);
kunmap_atomic(user_mem);
 
flush_dcache_page(page);
@@ -431,7 +442,7 @@ static ssize_t mm_stat_show(struct device *dev,
mem_used << PAGE_SHIFT,
zram->limit_pages << PAGE_SHIFT,
max_used << PAGE_SHIFT,
-   (u64)atomic64_read(>stats.zero_pages),
+   (u64)atomic64_read(>stats.same_pages),
pool_stats.pages_compacted);
up_read(>init_lock);
 
@@ -464,7 +475,7 @@ static ssize_t debug_stat_show(struct device *dev,
 ZRAM_ATTR_RO(failed_writes);
 ZRAM_ATTR_RO(invalid_io);
 ZRAM_ATTR_RO(notify_free);
-ZRAM_ATTR_RO(zero_pages);
+ZRAM_ATTR_RO(same_pages);
 ZRAM_ATTR_RO(compr_data_size);
 
 static inline bool zram_meta_get(struct zram *zram)
@@ -538,18 +549,20 @@ static void zram_free_page(struct zram *zram, size_t 
index)
struct zram_meta *meta = zram->meta;
unsigned long handle = meta->table[index].handle;
 
-   if (unlikely(!handle)) {
-   /*
-* No memory is allocated for zero filled pages.
-* Simply clear zero page flag.
-*/
-   if (zram_test_flag(meta, index, ZRAM_ZERO)) {
-   zram_clear_flag(meta, index, ZRAM_ZERO);
-   atomic64_dec(>stats.zero_pages);
-   }
+   /*
+* No memory is allocated for same element filled pages.
+* Simply clear same page flag.
+*/
+   if (zram_test_flag(meta, index, ZRAM_SAME)) {
+   zram_clear_flag(meta, index, ZRAM_SAME);
+   zram_clear_element(meta, index);
+   atomic64_dec(>stats.same_pages);
return;
}
 
+   if (!handle)
+   return;
+
zs_free(meta->mem_pool, handle);
 
atomic64_sub(zram_get_obj_size(meta, index),
@@ -572,9 +585,9 @@ static int zram_decompress_page(struct zram *zram, char 
*mem, u32 index)
handle = meta->table[index].handle;
size = zram_get_obj_size(meta, index);
 
-   if (!handle || zram_test_flag(meta, index, ZRAM_ZERO)) {
+   if (!handle || zram_test_flag(meta, index, ZRAM_SAME)) {
bit_spin_unlock(ZRAM_ACCESS, >table[index].value);
-   clear_page(mem);
+   memset(mem, (char)meta->table[index].element, PAGE_SIZE);
return 0;
}
 
@@ -610,9 +623,9 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec 
*bvec,
 
bit_spin_lock(ZRAM_ACCESS, >table[index].value);
if (unlikely(!meta->table[index].handle) ||
-   zram_test_flag(meta, index, ZRAM_ZERO)) {
+   zram_test_flag(meta, index, ZRAM_SAME)) {
bit_spin_unlock(ZRAM_ACCESS, >table[index].value);
-   handle_zero_page(bvec);
+   handle_same_page(bvec, 

Re: [PATCH v3 0/8] pstore: ramoops: support multiple pmsg instances

2017-01-03 Thread Kees Cook
On Mon, Dec 26, 2016 at 4:48 PM, 岩松信洋 / IWAMATSU,NOBUHIRO
 wrote:
> Ping?_
>
>> -Original Message-
>> From: linux-kernel-ow...@vger.kernel.org
>> [mailto:linux-kernel-ow...@vger.kernel.org] On Behalf Of 岩松信洋 /
>> IWAMATSU,NOBUHIRO
>> Sent: Monday, December 05, 2016 10:47 AM
>> To: Kees Cook
>> Cc: Anton Vorontsov; Colin Cross; Tony Luck; LKML;
>> cti.systems-productivity-manager...@hitachi.com
>> Subject: RE: [PATCH v3 0/8] pstore: ramoops: support multiple pmsg instances
>>
>> Hi, Kees.
>>
>> > -Original Message-
>> > From: keesc...@google.com [mailto:keesc...@google.com] On Behalf Of
>> > Kees Cook
>> > Sent: Saturday, November 12, 2016 7:24 AM
>> > To: 岩松信洋 / IWAMATSU,NOBUHIRO
>> > Cc: Anton Vorontsov; Colin Cross; Tony Luck; LKML;
>> > cti.systems-productivity-manager...@hitachi.com
>> > Subject: Re: [PATCH v3 0/8] pstore: ramoops: support multiple pmsg
>> > instances
>> >
>> > On Tue, Oct 18, 2016 at 12:13 AM, Nobuhiro Iwamatsu
>> >  wrote:
>> > > The following series implements multiple pmsg. This feature allows
>> > > userspace program to control individual content aging or priority.
>> > >
>> > > If a pstore backend module(e.g. ramoops) requires the multiple pmsg
>> > > instances when registering itself to pstore, multiple /dev/pmsg[ID]
>> > > are created. Writes to each /dev/pmsg[ID] are isolated each other.
>> > > After reboot, the contents are available in
>> > /sys/fs/pstore/pmsg-[backend]-[ID].
>> > >
>> > > In addition, we add multiple pmsg support for ramoops. We can
>> > > specify multiple pmsg area size by its module parameter as follows.
>> > >
>> > >  pmsg_size=0x1000,0x2000,...
>> > >
>> > > I did check the operation of this feature on CycloneV (socfpga)
>> > > Helio
>> > board.
>> > >
>> > > v3:
>> > >   Rebase to v4.8.
>> > >   Split patch.
>> > >   merged device_create().
>> > >   Remove Blank lines.
>> > >   Update documentiation of DT binding.
>> > >   Update parsing function of ramoops_pmsg_size, add NULL termination.
>> > >   Update module parameters for pmsg_size list.
>> >
>> > Thanks for this v3! Sorry for the delay, I should be able to review
>> > this shortly.
>>
>> Thank you.
>> I will wait for your review.

Now that the big changes have landed in Linus's tree, are you able to
rebase your series on those?

-Kees

-- 
Kees Cook
Nexus Security


Re: [PATCH v3 0/8] pstore: ramoops: support multiple pmsg instances

2017-01-03 Thread Kees Cook
On Mon, Dec 26, 2016 at 4:48 PM, 岩松信洋 / IWAMATSU,NOBUHIRO
 wrote:
> Ping?_
>
>> -Original Message-
>> From: linux-kernel-ow...@vger.kernel.org
>> [mailto:linux-kernel-ow...@vger.kernel.org] On Behalf Of 岩松信洋 /
>> IWAMATSU,NOBUHIRO
>> Sent: Monday, December 05, 2016 10:47 AM
>> To: Kees Cook
>> Cc: Anton Vorontsov; Colin Cross; Tony Luck; LKML;
>> cti.systems-productivity-manager...@hitachi.com
>> Subject: RE: [PATCH v3 0/8] pstore: ramoops: support multiple pmsg instances
>>
>> Hi, Kees.
>>
>> > -Original Message-
>> > From: keesc...@google.com [mailto:keesc...@google.com] On Behalf Of
>> > Kees Cook
>> > Sent: Saturday, November 12, 2016 7:24 AM
>> > To: 岩松信洋 / IWAMATSU,NOBUHIRO
>> > Cc: Anton Vorontsov; Colin Cross; Tony Luck; LKML;
>> > cti.systems-productivity-manager...@hitachi.com
>> > Subject: Re: [PATCH v3 0/8] pstore: ramoops: support multiple pmsg
>> > instances
>> >
>> > On Tue, Oct 18, 2016 at 12:13 AM, Nobuhiro Iwamatsu
>> >  wrote:
>> > > The following series implements multiple pmsg. This feature allows
>> > > userspace program to control individual content aging or priority.
>> > >
>> > > If a pstore backend module(e.g. ramoops) requires the multiple pmsg
>> > > instances when registering itself to pstore, multiple /dev/pmsg[ID]
>> > > are created. Writes to each /dev/pmsg[ID] are isolated each other.
>> > > After reboot, the contents are available in
>> > /sys/fs/pstore/pmsg-[backend]-[ID].
>> > >
>> > > In addition, we add multiple pmsg support for ramoops. We can
>> > > specify multiple pmsg area size by its module parameter as follows.
>> > >
>> > >  pmsg_size=0x1000,0x2000,...
>> > >
>> > > I did check the operation of this feature on CycloneV (socfpga)
>> > > Helio
>> > board.
>> > >
>> > > v3:
>> > >   Rebase to v4.8.
>> > >   Split patch.
>> > >   merged device_create().
>> > >   Remove Blank lines.
>> > >   Update documentiation of DT binding.
>> > >   Update parsing function of ramoops_pmsg_size, add NULL termination.
>> > >   Update module parameters for pmsg_size list.
>> >
>> > Thanks for this v3! Sorry for the delay, I should be able to review
>> > this shortly.
>>
>> Thank you.
>> I will wait for your review.

Now that the big changes have landed in Linus's tree, are you able to
rebase your series on those?

-Kees

-- 
Kees Cook
Nexus Security


[PATCH] checkpatch: Warn on embedded function names

2017-01-03 Thread Joe Perches
Embedded function names are less appropriate to use when
refactoring can cause function renaming.  Prefer the use
of "%s", __func__ to embedded function names.

Signed-off-by: Joe Perches 
---
 scripts/checkpatch.pl | 22 --
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 982c52ca6473..4f53093a1b0b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2134,7 +2134,7 @@ sub process {
my $in_header_lines = $file ? 0 : 1;
my $in_commit_log = 0;  #Scanning lines before patch
my $has_commit_log = 0; #Encountered lines before patch
-   my $commit_log_possible_stack_dump = 0;
+   my $commit_log_possible_stack_dump = 0;
my $commit_log_long_line = 0;
my $commit_log_has_diff = 0;
my $reported_maintainer_file = 0;
@@ -2154,6 +2154,7 @@ sub process {
my $realline = 0;
my $realcnt = 0;
my $here = '';
+   my $context_function;   #undef'd unless there's a known function
my $in_comment = 0;
my $comment_edge = 0;
my $first_line = 0;
@@ -2192,7 +2193,8 @@ sub process {
}
#next;
}
-   if ($rawline=~/^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@/) {
+   if ($rawline=~/^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@(.*)/) {
+   my $context = $4;
$realline=$1-1;
if (defined $2) {
$realcnt=$3+1;
@@ -2201,6 +2203,12 @@ sub process {
}
$in_comment = 0;
 
+   if ($context =~ /\b(\w+)\s*\(/) {
+   $context_function = $1;
+   } else {
+   undef $context_function;
+   }
+
# Guestimate if this is a continuing comment.  Run
# the context looking for a comment "edge".  If this
# edge is a close comment then we must be in a comment
@@ -5157,6 +5165,16 @@ sub process {
 "break quoted strings at a space character\n" . 
$hereprev);
}
 
+#check for an embedded function name in a string when the function is known
+# as part of a diff.  This does not work for -f --file checking as it
+#depends on patch context providing the function name
+   if ($line =~ /^\+.*$String/ &&
+   defined($context_function) &&
+   get_quoted_string($line, $rawline) =~ 
/\b$context_function\b/) {
+   WARN("EMBEDDED_FUNCTION_NAME",
+"Prefer using \"%s\", __func__ to embedded 
function names\n" . $herecurr);
+   }
+
 # check for spaces before a quoted newline
if ($rawline =~ /^.*\".*\s\\n/) {
if (WARN("QUOTED_WHITESPACE_BEFORE_NEWLINE",
-- 
2.10.0.rc2.1.g053435c



[PATCH] checkpatch: Warn on embedded function names

2017-01-03 Thread Joe Perches
Embedded function names are less appropriate to use when
refactoring can cause function renaming.  Prefer the use
of "%s", __func__ to embedded function names.

Signed-off-by: Joe Perches 
---
 scripts/checkpatch.pl | 22 --
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 982c52ca6473..4f53093a1b0b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2134,7 +2134,7 @@ sub process {
my $in_header_lines = $file ? 0 : 1;
my $in_commit_log = 0;  #Scanning lines before patch
my $has_commit_log = 0; #Encountered lines before patch
-   my $commit_log_possible_stack_dump = 0;
+   my $commit_log_possible_stack_dump = 0;
my $commit_log_long_line = 0;
my $commit_log_has_diff = 0;
my $reported_maintainer_file = 0;
@@ -2154,6 +2154,7 @@ sub process {
my $realline = 0;
my $realcnt = 0;
my $here = '';
+   my $context_function;   #undef'd unless there's a known function
my $in_comment = 0;
my $comment_edge = 0;
my $first_line = 0;
@@ -2192,7 +2193,8 @@ sub process {
}
#next;
}
-   if ($rawline=~/^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@/) {
+   if ($rawline=~/^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@(.*)/) {
+   my $context = $4;
$realline=$1-1;
if (defined $2) {
$realcnt=$3+1;
@@ -2201,6 +2203,12 @@ sub process {
}
$in_comment = 0;
 
+   if ($context =~ /\b(\w+)\s*\(/) {
+   $context_function = $1;
+   } else {
+   undef $context_function;
+   }
+
# Guestimate if this is a continuing comment.  Run
# the context looking for a comment "edge".  If this
# edge is a close comment then we must be in a comment
@@ -5157,6 +5165,16 @@ sub process {
 "break quoted strings at a space character\n" . 
$hereprev);
}
 
+#check for an embedded function name in a string when the function is known
+# as part of a diff.  This does not work for -f --file checking as it
+#depends on patch context providing the function name
+   if ($line =~ /^\+.*$String/ &&
+   defined($context_function) &&
+   get_quoted_string($line, $rawline) =~ 
/\b$context_function\b/) {
+   WARN("EMBEDDED_FUNCTION_NAME",
+"Prefer using \"%s\", __func__ to embedded 
function names\n" . $herecurr);
+   }
+
 # check for spaces before a quoted newline
if ($rawline =~ /^.*\".*\s\\n/) {
if (WARN("QUOTED_WHITESPACE_BEFORE_NEWLINE",
-- 
2.10.0.rc2.1.g053435c



Re: [PATCH RFC V2] purgatory: fix up declarations

2017-01-03 Thread Dave Young
Vivek, thanks for ccing me..

On 01/03/17 at 04:34pm, Nicholas Mc Guire wrote:
> On Tue, Jan 03, 2017 at 10:38:14AM -0500, Vivek Goyal wrote:
> > On Fri, Dec 23, 2016 at 12:43:07PM +0100, Nicholas Mc Guire wrote:
> > > Add the missing declarations of basic purgatory functions and variables
> > > used with kexec_purgatory_get_set_symbol() to allow a clean build.
> > > 
> > > Fixes: commit 8fc5b4d4121c ("purgatory: core purgatory functionality")
> > > Signed-off-by: Nicholas Mc Guire 
> > > ---
> > > 
> > > V2: after kbuild test robot  reported a build failure
> > > removed incorrect declaration of copy_backup_region which is static
> > > anyway.
> > > 
> > > sparse complained about:
> > >   CHECK   arch/x86/purgatory/purgatory.c
> > > arch/x86/purgatory/purgatory.c:21:15: warning: symbol 'backup_dest' was 
> > > not declared. Should it be static?
> > > arch/x86/purgatory/purgatory.c:22:15: warning: symbol 'backup_src' was 
> > > not declared. Should it be static?
> > > arch/x86/purgatory/purgatory.c:23:15: warning: symbol 'backup_sz' was not 
> > > declared. Should it be static?
> > > arch/x86/purgatory/purgatory.c:25:4: warning: symbol 'sha256_digest' was 
> > > not declared. Should it be static?
> > > arch/x86/purgatory/purgatory.c:27:19: warning: symbol 'sha_regions' was 
> > > not declared. Should it be static?
> > > arch/x86/purgatory/purgatory.c:42:5: warning: symbol 
> > > 'verify_sha256_digest' was not declared. Should it be static?
> > > arch/x86/purgatory/purgatory.c:61:6: warning: symbol 'purgatory' was not 
> > > declared. Should it be static?
> > >   CC  arch/x86/purgatory/purgatory.o
> > > 
> > > Numerous sparse messages regarding functions not being declared, these
> > > functions are resolved via kexec_purgatory_get_set_symbol() and not
> > > directly called anywhere.
> > 
> > Hi Nicholas,
> > 
> > So purgatory() is a separate piece of small binary which does not link
> > against kernel. And we don't want these symbols static as kernel
> > obtains the values of these symbols and modifies binary in place on
> > the fly. I am assuming if we make them static, then we will lose this
> > capability to be able to read elf headers and be able to modify value
> > of these symbols.
> 
> I don´t understand why this would be lost - the symbols are not being
> used by kernel code other than kexec code it self -  in what way
> would declaring them extern change there handling ? 
> kexec_purgatory_find_symbol is using the elf header to resolve the 
> symbol location and declaring it extern should not change that in any 
> way - am I overlooking something ?
> 
> > 
> > Now question is how to supress warnings from sparse. If just declaring
> > them extern in header file and including that header file in some other
> > .c file make the sparse warning go away, so be it. Atleast we need
> > to make explicit comment that this is being done just to take care
> > of sparse warning.
> > 
> > I am not very happy with the solution though. In future it will make
> > people scratch their head that why are we including this header file
> > and why some symbols are being declared extern. So if there is another
> > way to tell sparse to not worry about it, would be even better.
> >
> 
> The assumtion was that these changes would be side-effect free - if they are
> not then this is probably the wrong path to go - the intent is to remove 
> the sparse warnings only.

Another way is do not include the header file, but declare them in the c
file just for avoiding the sparse warnings with some comments to explain
it.

>  
> > 
> > > To resolve the sparse issues appropriate
> > > declarations were added to asm/kexec-bzimage64.h and the appropriate
> > > reference included in purgatory.c. Adding this to kexec-bzimage64.h
> > > was done as setup_purgatory() from machine_kexec_file_64.c uses
> > > these symbols - not sure if this is the proper place to add this.
> > > 
> > > While at it the initialization of sha_regions to {{0,0}} was added.
> > > 
> > 
> > Is it really required. I thought all these global variable are will be
> > set to 0 without doing anything explicit.
> >
> 
> No - technically it is not needed - but rather just a consistency
> issue as 
> u8 sha256_digest[SHA256_DIGEST_SIZE] = { 0 };
> is being initialized explicidly I just found it consostent to initialize
>struct sha_region sha_regions[16] = {};
> explicidly as well - but that can be dropped if its usual practice.

It can be in another patch, removing the explicit zero initializing
looks better..

> 
> thx!
> hofrat
>  
> > Vivek
> > 
> > > Patch was compile tested with: x86_64_defconfig (implies CONFIG_KEXEC=y)
> > > 
> > > Patch is against 4.9.0 (localversion-next is next-20161223)
> > > 
> > >  arch/x86/include/asm/kexec-bzimage64.h | 16 
> > >  arch/x86/purgatory/purgatory.c |  8 ++--
> > >  2 files changed, 18 insertions(+), 6 deletions(-)
> > > 
> > > diff --git 

[PATCH] checkpatch: Warn on embedded function names

2017-01-03 Thread Joe Perches
Embedded function names are less appropriate to use when
refactoring can cause function renaming.  Prefer the use
of "%s", __func__ to embedded function names.

Signed-off-by: Joe Perches 
---
 scripts/checkpatch.pl | 22 --
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 982c52ca6473..4f53093a1b0b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2134,7 +2134,7 @@ sub process {
my $in_header_lines = $file ? 0 : 1;
my $in_commit_log = 0;  #Scanning lines before patch
my $has_commit_log = 0; #Encountered lines before patch
-   my $commit_log_possible_stack_dump = 0;
+   my $commit_log_possible_stack_dump = 0;
my $commit_log_long_line = 0;
my $commit_log_has_diff = 0;
my $reported_maintainer_file = 0;
@@ -2154,6 +2154,7 @@ sub process {
my $realline = 0;
my $realcnt = 0;
my $here = '';
+   my $context_function;   #undef'd unless there's a known function
my $in_comment = 0;
my $comment_edge = 0;
my $first_line = 0;
@@ -2192,7 +2193,8 @@ sub process {
}
#next;
}
-   if ($rawline=~/^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@/) {
+   if ($rawline=~/^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@(.*)/) {
+   my $context = $4;
$realline=$1-1;
if (defined $2) {
$realcnt=$3+1;
@@ -2201,6 +2203,12 @@ sub process {
}
$in_comment = 0;
 
+   if ($context =~ /\b(\w+)\s*\(/) {
+   $context_function = $1;
+   } else {
+   undef $context_function;
+   }
+
# Guestimate if this is a continuing comment.  Run
# the context looking for a comment "edge".  If this
# edge is a close comment then we must be in a comment
@@ -5157,6 +5165,16 @@ sub process {
 "break quoted strings at a space character\n" . 
$hereprev);
}
 
+#check for an embedded function name in a string when the function is known
+# as part of a diff.  This does not work for -f --file checking as it
+#depends on patch context providing the function name
+   if ($line =~ /^\+.*$String/ &&
+   defined($context_function) &&
+   get_quoted_string($line, $rawline) =~ 
/\b$context_function\b/) {
+   WARN("EMBEDDED_FUNCTION_NAME",
+"Prefer using \"%s\", __func__ to embedded 
function names\n" . $herecurr);
+   }
+
 # check for spaces before a quoted newline
if ($rawline =~ /^.*\".*\s\\n/) {
if (WARN("QUOTED_WHITESPACE_BEFORE_NEWLINE",
-- 
2.10.0.rc2.1.g053435c



Re: [PATCH RFC V2] purgatory: fix up declarations

2017-01-03 Thread Dave Young
Vivek, thanks for ccing me..

On 01/03/17 at 04:34pm, Nicholas Mc Guire wrote:
> On Tue, Jan 03, 2017 at 10:38:14AM -0500, Vivek Goyal wrote:
> > On Fri, Dec 23, 2016 at 12:43:07PM +0100, Nicholas Mc Guire wrote:
> > > Add the missing declarations of basic purgatory functions and variables
> > > used with kexec_purgatory_get_set_symbol() to allow a clean build.
> > > 
> > > Fixes: commit 8fc5b4d4121c ("purgatory: core purgatory functionality")
> > > Signed-off-by: Nicholas Mc Guire 
> > > ---
> > > 
> > > V2: after kbuild test robot  reported a build failure
> > > removed incorrect declaration of copy_backup_region which is static
> > > anyway.
> > > 
> > > sparse complained about:
> > >   CHECK   arch/x86/purgatory/purgatory.c
> > > arch/x86/purgatory/purgatory.c:21:15: warning: symbol 'backup_dest' was 
> > > not declared. Should it be static?
> > > arch/x86/purgatory/purgatory.c:22:15: warning: symbol 'backup_src' was 
> > > not declared. Should it be static?
> > > arch/x86/purgatory/purgatory.c:23:15: warning: symbol 'backup_sz' was not 
> > > declared. Should it be static?
> > > arch/x86/purgatory/purgatory.c:25:4: warning: symbol 'sha256_digest' was 
> > > not declared. Should it be static?
> > > arch/x86/purgatory/purgatory.c:27:19: warning: symbol 'sha_regions' was 
> > > not declared. Should it be static?
> > > arch/x86/purgatory/purgatory.c:42:5: warning: symbol 
> > > 'verify_sha256_digest' was not declared. Should it be static?
> > > arch/x86/purgatory/purgatory.c:61:6: warning: symbol 'purgatory' was not 
> > > declared. Should it be static?
> > >   CC  arch/x86/purgatory/purgatory.o
> > > 
> > > Numerous sparse messages regarding functions not being declared, these
> > > functions are resolved via kexec_purgatory_get_set_symbol() and not
> > > directly called anywhere.
> > 
> > Hi Nicholas,
> > 
> > So purgatory() is a separate piece of small binary which does not link
> > against kernel. And we don't want these symbols static as kernel
> > obtains the values of these symbols and modifies binary in place on
> > the fly. I am assuming if we make them static, then we will lose this
> > capability to be able to read elf headers and be able to modify value
> > of these symbols.
> 
> I don´t understand why this would be lost - the symbols are not being
> used by kernel code other than kexec code it self -  in what way
> would declaring them extern change there handling ? 
> kexec_purgatory_find_symbol is using the elf header to resolve the 
> symbol location and declaring it extern should not change that in any 
> way - am I overlooking something ?
> 
> > 
> > Now question is how to supress warnings from sparse. If just declaring
> > them extern in header file and including that header file in some other
> > .c file make the sparse warning go away, so be it. Atleast we need
> > to make explicit comment that this is being done just to take care
> > of sparse warning.
> > 
> > I am not very happy with the solution though. In future it will make
> > people scratch their head that why are we including this header file
> > and why some symbols are being declared extern. So if there is another
> > way to tell sparse to not worry about it, would be even better.
> >
> 
> The assumtion was that these changes would be side-effect free - if they are
> not then this is probably the wrong path to go - the intent is to remove 
> the sparse warnings only.

Another way is do not include the header file, but declare them in the c
file just for avoiding the sparse warnings with some comments to explain
it.

>  
> > 
> > > To resolve the sparse issues appropriate
> > > declarations were added to asm/kexec-bzimage64.h and the appropriate
> > > reference included in purgatory.c. Adding this to kexec-bzimage64.h
> > > was done as setup_purgatory() from machine_kexec_file_64.c uses
> > > these symbols - not sure if this is the proper place to add this.
> > > 
> > > While at it the initialization of sha_regions to {{0,0}} was added.
> > > 
> > 
> > Is it really required. I thought all these global variable are will be
> > set to 0 without doing anything explicit.
> >
> 
> No - technically it is not needed - but rather just a consistency
> issue as 
> u8 sha256_digest[SHA256_DIGEST_SIZE] = { 0 };
> is being initialized explicidly I just found it consostent to initialize
>struct sha_region sha_regions[16] = {};
> explicidly as well - but that can be dropped if its usual practice.

It can be in another patch, removing the explicit zero initializing
looks better..

> 
> thx!
> hofrat
>  
> > Vivek
> > 
> > > Patch was compile tested with: x86_64_defconfig (implies CONFIG_KEXEC=y)
> > > 
> > > Patch is against 4.9.0 (localversion-next is next-20161223)
> > > 
> > >  arch/x86/include/asm/kexec-bzimage64.h | 16 
> > >  arch/x86/purgatory/purgatory.c |  8 ++--
> > >  2 files changed, 18 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/arch/x86/include/asm/kexec-bzimage64.h 

[PATCH] checkpatch: Warn on embedded function names

2017-01-03 Thread Joe Perches
Embedded function names are less appropriate to use when
refactoring can cause function renaming.  Prefer the use
of "%s", __func__ to embedded function names.

Signed-off-by: Joe Perches 
---
 scripts/checkpatch.pl | 22 --
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 982c52ca6473..4f53093a1b0b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2134,7 +2134,7 @@ sub process {
my $in_header_lines = $file ? 0 : 1;
my $in_commit_log = 0;  #Scanning lines before patch
my $has_commit_log = 0; #Encountered lines before patch
-   my $commit_log_possible_stack_dump = 0;
+   my $commit_log_possible_stack_dump = 0;
my $commit_log_long_line = 0;
my $commit_log_has_diff = 0;
my $reported_maintainer_file = 0;
@@ -2154,6 +2154,7 @@ sub process {
my $realline = 0;
my $realcnt = 0;
my $here = '';
+   my $context_function;   #undef'd unless there's a known function
my $in_comment = 0;
my $comment_edge = 0;
my $first_line = 0;
@@ -2192,7 +2193,8 @@ sub process {
}
#next;
}
-   if ($rawline=~/^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@/) {
+   if ($rawline=~/^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@(.*)/) {
+   my $context = $4;
$realline=$1-1;
if (defined $2) {
$realcnt=$3+1;
@@ -2201,6 +2203,12 @@ sub process {
}
$in_comment = 0;
 
+   if ($context =~ /\b(\w+)\s*\(/) {
+   $context_function = $1;
+   } else {
+   undef $context_function;
+   }
+
# Guestimate if this is a continuing comment.  Run
# the context looking for a comment "edge".  If this
# edge is a close comment then we must be in a comment
@@ -5157,6 +5165,16 @@ sub process {
 "break quoted strings at a space character\n" . 
$hereprev);
}
 
+#check for an embedded function name in a string when the function is known
+# as part of a diff.  This does not work for -f --file checking as it
+#depends on patch context providing the function name
+   if ($line =~ /^\+.*$String/ &&
+   defined($context_function) &&
+   get_quoted_string($line, $rawline) =~ 
/\b$context_function\b/) {
+   WARN("EMBEDDED_FUNCTION_NAME",
+"Prefer using \"%s\", __func__ to embedded 
function names\n" . $herecurr);
+   }
+
 # check for spaces before a quoted newline
if ($rawline =~ /^.*\".*\s\\n/) {
if (WARN("QUOTED_WHITESPACE_BEFORE_NEWLINE",
-- 
2.10.0.rc2.1.g053435c



  1   2   3   4   5   6   7   8   9   10   >