[PATCH v2] staging: mt7621-dma: Add braces around else branches

2018-10-23 Thread Kimberly Brown 
Add braces around else branches in conditional statements that include
branches with multiple statements. This style complies with the Linux
kernel coding style and improves consistency and readability. Issues found by
checkpatch.

Signed-off-by: Kimberly Brown 
Reviewed-by: Matthias Brugger 
---
Changes in v2:
- Added people and mailing lists from get_maintainer.pl to the CC list
- Added Reviewed-by line

 drivers/staging/mt7621-dma/mtk-hsdma.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/mt7621-dma/mtk-hsdma.c 
b/drivers/staging/mt7621-dma/mtk-hsdma.c
index df6ebf41bdea..35556f024aa1 100644
--- a/drivers/staging/mt7621-dma/mtk-hsdma.c
+++ b/drivers/staging/mt7621-dma/mtk-hsdma.c
@@ -418,8 +418,9 @@ static void mtk_hsdma_chan_done(struct mtk_hsdam_engine 
*hsdma,
vchan_cookie_complete(>vdesc);
chan_issued = gdma_next_desc(chan);
}
-   } else
+   } else {
dev_dbg(hsdma->ddev.dev, "no desc to complete\n");
+   }
 
if (chan_issued)
set_bit(chan->id, >chan_issued);
@@ -456,8 +457,9 @@ static void mtk_hsdma_issue_pending(struct dma_chan *c)
if (gdma_next_desc(chan)) {
set_bit(chan->id, >chan_issued);
tasklet_schedule(>task);
-   } else
+   } else {
dev_dbg(hsdma->ddev.dev, "no desc to issue\n");
+   }
}
spin_unlock_bh(>vchan.lock);
 }
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/3] staging: greybus: loopback.c: do insertion in O(n) instead of O(n lg n)

2018-10-23 Thread Bryan O'Donoghue 

On 23/10/2018 17:20, Rasmus Villemoes wrote:

On 2018-10-11 01:03, Bryan O'Donoghue wrote:

On 05/10/2018 15:28, Rasmus Villemoes wrote:

Signed-off-by: Rasmus Villemoes
---
I have no idea if the performance matters (it probably doesn't). Feel
free to ignore this and the followup cleanup.


What's the problem you're fixing here ?

Is it tested ?


I got curious why one would want to keep a linked list sorted in the
first place (it can't be for doing binary searches). But it seems that
gb_loopback_device::list is unused, along with the list_op_async. Given
that the below compiles, doesn't that prove that the code is
dead/unused, or what am I missing?

diff --git a/drivers/staging/greybus/loopback.c
b/drivers/staging/greybus/loopback.c
index 7080294f705c..e4d42c1dc284 100644
--- a/drivers/staging/greybus/loopback.c
+++ b/drivers/staging/greybus/loopback.c
@@ -47,8 +47,6 @@ struct gb_loopback_device {

/* We need to take a lock in atomic context */
spinlock_t lock;
-   struct list_head list;
-   struct list_head list_op_async;
wait_queue_head_t wq;
  };

@@ -68,7 +66,6 @@ struct gb_loopback {
struct kfifo kfifo_lat;
struct mutex mutex;
struct task_struct *task;
-   struct list_head entry;
struct device *dev;
wait_queue_head_t wq;
wait_queue_head_t wq_completion;
@@ -987,37 +984,6 @@ static const struct file_operations
gb_loopback_debugfs_latency_ops = {
.release= single_release,
  };

-static int gb_loopback_bus_id_compare(void *priv, struct list_head *lha,
- struct list_head *lhb)
-{
-   struct gb_loopback *a = list_entry(lha, struct gb_loopback, entry);
-   struct gb_loopback *b = list_entry(lhb, struct gb_loopback, entry);
-   struct gb_connection *ca = a->connection;
-   struct gb_connection *cb = b->connection;
-
-   if (ca->bundle->intf->interface_id < cb->bundle->intf->interface_id)
-   return -1;
-   if (cb->bundle->intf->interface_id < ca->bundle->intf->interface_id)
-   return 1;
-   if (ca->bundle->id < cb->bundle->id)
-   return -1;
-   if (cb->bundle->id < ca->bundle->id)
-   return 1;
-   if (ca->intf_cport_id < cb->intf_cport_id)
-   return -1;
-   else if (cb->intf_cport_id < ca->intf_cport_id)
-   return 1;
-
-   return 0;
-}
-
-static void gb_loopback_insert_id(struct gb_loopback *gb)
-{
-   /* perform an insertion sort */
-   list_add_tail(>entry, _dev.list);
-   list_sort(NULL, _dev.list, gb_loopback_bus_id_compare);
-}
-
  #define DEBUGFS_NAMELEN 32

  static int gb_loopback_probe(struct gb_bundle *bundle,
@@ -1113,7 +1079,6 @@ static int gb_loopback_probe(struct gb_bundle *bundle,
}

spin_lock_irqsave(_dev.lock, flags);
-   gb_loopback_insert_id(gb);
gb_dev.count++;
spin_unlock_irqrestore(_dev.lock, flags);

@@ -1169,7 +1134,6 @@ static void gb_loopback_disconnect(struct
gb_bundle *bundle)

spin_lock_irqsave(_dev.lock, flags);
gb_dev.count--;
-   list_del(>entry);
spin_unlock_irqrestore(_dev.lock, flags);

device_unregister(gb->dev);
@@ -1196,8 +1160,6 @@ static int loopback_init(void)
  {
int retval;

-   INIT_LIST_HEAD(_dev.list);
-   INIT_LIST_HEAD(_dev.list_op_async);
spin_lock_init(_dev.lock);
gb_dev.root = debugfs_create_dir("gb_loopback", NULL);



Looks like dead code alright.

Reviewed-by: Bryan O'Donoghue 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3] staging: iio: ad2s1210: Switch to the gpio descriptor interface

2018-10-23 Thread Nishad Kamdar 
Use the gpiod interface instead of the deprecated old non-descriptor
interface.

Signed-off-by: Nishad Kamdar 
---
Changes in v3:
 - Use a pointer to pointer for gpio_desc in
   struct ad2s1210_gpio as it will be used to
   modify a pointer.
 - Use dot notation to initialize the structure.
 - Use a pointer variable to avoid writing gpios[i].
Changes in v2:
 - Use the spi_device struct embedded in st instead
   of passing it as an argument to ad2s1210_setup_gpios().
 - Use an array of structs to reduce redundant code in
   in ad2s1210_setup_gpios().
 - Remove ad2s1210_free_gpios() as devm API is being used.
---
 drivers/staging/iio/resolver/ad2s1210.c | 92 ++---
 drivers/staging/iio/resolver/ad2s1210.h |  3 -
 2 files changed, 50 insertions(+), 45 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1210.c 
b/drivers/staging/iio/resolver/ad2s1210.c
index ac13b99bd9cb..fbf0cc722979 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -15,7 +15,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 
 #include 
@@ -69,10 +69,21 @@ enum ad2s1210_mode {
 
 static const unsigned int ad2s1210_resolution_value[] = { 10, 12, 14, 16 };
 
+struct ad2s1210_gpio {
+   struct gpio_desc **ptr;
+   const char *name;
+   unsigned long flags;
+};
+
 struct ad2s1210_state {
const struct ad2s1210_platform_data *pdata;
struct mutex lock;
struct spi_device *sdev;
+   struct gpio_desc *sample;
+   struct gpio_desc *a0;
+   struct gpio_desc *a1;
+   struct gpio_desc *res0;
+   struct gpio_desc *res1;
unsigned int fclkin;
unsigned int fexcit;
bool hysteresis;
@@ -91,8 +102,8 @@ static const int ad2s1210_mode_vals[4][2] = {
 static inline void ad2s1210_set_mode(enum ad2s1210_mode mode,
 struct ad2s1210_state *st)
 {
-   gpio_set_value(st->pdata->a[0], ad2s1210_mode_vals[mode][0]);
-   gpio_set_value(st->pdata->a[1], ad2s1210_mode_vals[mode][1]);
+   gpiod_set_value(st->a0, ad2s1210_mode_vals[mode][0]);
+   gpiod_set_value(st->a1, ad2s1210_mode_vals[mode][1]);
st->mode = mode;
 }
 
@@ -152,8 +163,8 @@ int ad2s1210_update_frequency_control_word(struct 
ad2s1210_state *st)
 
 static unsigned char ad2s1210_read_resolution_pin(struct ad2s1210_state *st)
 {
-   int resolution = (gpio_get_value(st->pdata->res[0]) << 1) |
- gpio_get_value(st->pdata->res[1]);
+   int resolution = (gpiod_get_value(st->res0) << 1) |
+ gpiod_get_value(st->res1);
 
return ad2s1210_resolution_value[resolution];
 }
@@ -164,10 +175,10 @@ static const int ad2s1210_res_pins[4][2] = {
 
 static inline void ad2s1210_set_resolution_pin(struct ad2s1210_state *st)
 {
-   gpio_set_value(st->pdata->res[0],
-  ad2s1210_res_pins[(st->resolution - 10) / 2][0]);
-   gpio_set_value(st->pdata->res[1],
-  ad2s1210_res_pins[(st->resolution - 10) / 2][1]);
+   gpiod_set_value(st->res0,
+   ad2s1210_res_pins[(st->resolution - 10) / 2][0]);
+   gpiod_set_value(st->res1,
+   ad2s1210_res_pins[(st->resolution - 10) / 2][1]);
 }
 
 static inline int ad2s1210_soft_reset(struct ad2s1210_state *st)
@@ -401,15 +412,15 @@ static ssize_t ad2s1210_clear_fault(struct device *dev,
int ret;
 
mutex_lock(>lock);
-   gpio_set_value(st->pdata->sample, 0);
+   gpiod_set_value(st->sample, 0);
/* delay (2 * tck + 20) nano seconds */
udelay(1);
-   gpio_set_value(st->pdata->sample, 1);
+   gpiod_set_value(st->sample, 1);
ret = ad2s1210_config_read(st, AD2S1210_REG_FAULT);
if (ret < 0)
goto error_ret;
-   gpio_set_value(st->pdata->sample, 0);
-   gpio_set_value(st->pdata->sample, 1);
+   gpiod_set_value(st->sample, 0);
+   gpiod_set_value(st->sample, 1);
 error_ret:
mutex_unlock(>lock);
 
@@ -466,7 +477,7 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
s16 vel;
 
mutex_lock(>lock);
-   gpio_set_value(st->pdata->sample, 0);
+   gpiod_set_value(st->sample, 0);
/* delay (6 * tck + 20) nano seconds */
udelay(1);
 
@@ -512,7 +523,7 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
}
 
 error_ret:
-   gpio_set_value(st->pdata->sample, 1);
+   gpiod_set_value(st->sample, 1);
/* delay (2 * tck + 20) nano seconds */
udelay(1);
mutex_unlock(>lock);
@@ -630,30 +641,32 @@ static const struct iio_info ad2s1210_info = {
 
 static int ad2s1210_setup_gpios(struct ad2s1210_state *st)
 {
-   unsigned long flags = st->pdata->gpioin ? GPIOF_DIR_IN : GPIOF_DIR_OUT;
-   struct gpio ad2s1210_gpios[] = {
-   { st->pdata->sample, GPIOF_DIR_IN, "sample" },
-   { st->pdata->a[0], flags, "a0" },
-   {

Re: [PATCH v2] staging: iio: ad2s1210: Switch to the gpio descriptor interface

2018-10-23 Thread Nishad Kamdar 
On Tue, Oct 23, 2018 at 01:13:56PM +0200, Slawomir Stepien wrote:
> On paź 22, 2018 22:09, Nishad Kamdar wrote:
> > Use the gpiod interface instead of the deprecated old non-descriptor
> > interface.
> 
> Hi
> 
> > Signed-off-by: Nishad Kamdar 
> > ---
> > Changes in v2:
> >  - Use the spi_device struct embedded in st instead
> >of passing it as an argument to ad2s1210_setup_gpios().
> >  - Use an array of structs to reduce redundant code in
> >in ad2s1210_setup_gpios().
> >  - Remove ad2s1210_free_gpios() as devm API is being used.
> 
> Some few more comments on that changes below.
> 
> > ---
> >  drivers/staging/iio/resolver/ad2s1210.c | 91 +
> >  drivers/staging/iio/resolver/ad2s1210.h |  3 -
> >  2 files changed, 48 insertions(+), 46 deletions(-)
> > 
> > diff --git a/drivers/staging/iio/resolver/ad2s1210.c 
> > b/drivers/staging/iio/resolver/ad2s1210.c
> > index ac13b99bd9cb..e4c222b47574 100644
> > --- a/drivers/staging/iio/resolver/ad2s1210.c
> > +++ b/drivers/staging/iio/resolver/ad2s1210.c
> > @@ -15,7 +15,7 @@
> >  #include 
> >  #include 
> >  #include 
> > -#include 
> > +#include 
> >  #include 
> >  
> >  #include 
> > @@ -69,10 +69,21 @@ enum ad2s1210_mode {
> >  
> >  static const unsigned int ad2s1210_resolution_value[] = { 10, 12, 14, 16 };
> >  
> > +struct ad2s1210_gpio {
> > +   struct gpio_desc *ptr;
> 
> You are going to use this ptr to change a pointer it points to. So it must be 
> a
> pointer to pointer: **ptr;
> 
> > +   char *name;
> 
> I guess it should be const char *.
> 
> > +   unsigned long flags;
> > +};
> > +
> >  struct ad2s1210_state {
> > const struct ad2s1210_platform_data *pdata;
> > struct mutex lock;
> > struct spi_device *sdev;
> > +   struct gpio_desc *sample;
> > +   struct gpio_desc *a0;
> > +   struct gpio_desc *a1;
> > +   struct gpio_desc *res0;
> > +   struct gpio_desc *res1;
> > unsigned int fclkin;
> > unsigned int fexcit;
> > bool hysteresis;
> > @@ -91,8 +102,8 @@ static const int ad2s1210_mode_vals[4][2] = {
> >  static inline void ad2s1210_set_mode(enum ad2s1210_mode mode,
> >  struct ad2s1210_state *st)
> >  {
> > -   gpio_set_value(st->pdata->a[0], ad2s1210_mode_vals[mode][0]);
> > -   gpio_set_value(st->pdata->a[1], ad2s1210_mode_vals[mode][1]);
> > +   gpiod_set_value(st->a0, ad2s1210_mode_vals[mode][0]);
> > +   gpiod_set_value(st->a1, ad2s1210_mode_vals[mode][1]);
> > st->mode = mode;
> >  }
> >  
> > @@ -152,8 +163,8 @@ int ad2s1210_update_frequency_control_word(struct 
> > ad2s1210_state *st)
> >  
> >  static unsigned char ad2s1210_read_resolution_pin(struct ad2s1210_state 
> > *st)
> >  {
> > -   int resolution = (gpio_get_value(st->pdata->res[0]) << 1) |
> > - gpio_get_value(st->pdata->res[1]);
> > +   int resolution = (gpiod_get_value(st->res0) << 1) |
> > + gpiod_get_value(st->res1);
> >  
> > return ad2s1210_resolution_value[resolution];
> >  }
> > @@ -164,10 +175,10 @@ static const int ad2s1210_res_pins[4][2] = {
> >  
> >  static inline void ad2s1210_set_resolution_pin(struct ad2s1210_state *st)
> >  {
> > -   gpio_set_value(st->pdata->res[0],
> > -  ad2s1210_res_pins[(st->resolution - 10) / 2][0]);
> > -   gpio_set_value(st->pdata->res[1],
> > -  ad2s1210_res_pins[(st->resolution - 10) / 2][1]);
> > +   gpiod_set_value(st->res0,
> > +   ad2s1210_res_pins[(st->resolution - 10) / 2][0]);
> > +   gpiod_set_value(st->res1,
> > +   ad2s1210_res_pins[(st->resolution - 10) / 2][1]);
> >  }
> >  
> >  static inline int ad2s1210_soft_reset(struct ad2s1210_state *st)
> > @@ -401,15 +412,15 @@ static ssize_t ad2s1210_clear_fault(struct device 
> > *dev,
> > int ret;
> >  
> > mutex_lock(>lock);
> > -   gpio_set_value(st->pdata->sample, 0);
> > +   gpiod_set_value(st->sample, 0);
> > /* delay (2 * tck + 20) nano seconds */
> > udelay(1);
> > -   gpio_set_value(st->pdata->sample, 1);
> > +   gpiod_set_value(st->sample, 1);
> > ret = ad2s1210_config_read(st, AD2S1210_REG_FAULT);
> > if (ret < 0)
> > goto error_ret;
> > -   gpio_set_value(st->pdata->sample, 0);
> > -   gpio_set_value(st->pdata->sample, 1);
> > +   gpiod_set_value(st->sample, 0);
> > +   gpiod_set_value(st->sample, 1);
> >  error_ret:
> > mutex_unlock(>lock);
> >  
> > @@ -466,7 +477,7 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
> > s16 vel;
> >  
> > mutex_lock(>lock);
> > -   gpio_set_value(st->pdata->sample, 0);
> > +   gpiod_set_value(st->sample, 0);
> > /* delay (6 * tck + 20) nano seconds */
> > udelay(1);
> >  
> > @@ -512,7 +523,7 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
> > }
> >  
> >  error_ret:
> > -   gpio_set_value(st->pdata->sample, 1);
> > +   gpiod_set_value(st->sample, 1);
> > /* delay (2 * tck + 20) nano seconds */
> > udelay(1);
> > mutex_unlock(>lock);
> > @@

[PATCH v2] staging: vboxvideo: Remove unnecessary parentheses

2018-10-23 Thread Shayenne da Luz Moura 
Remove unneeded parentheses around the arguments of ||. This reduces
clutter and code behave in the same way.
Change suggested by checkpatch.pl.

vbox_main.c:119: CHECK: Unnecessary parentheses around 'rects[i].x2 <
crtc->x'

Signed-off-by: Shayenne da Luz Moura 
---
Changes in v2:
  - Make the commit message more clearer.
 
 drivers/staging/vboxvideo/vbox_main.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/vboxvideo/vbox_main.c 
b/drivers/staging/vboxvideo/vbox_main.c
index 429f6a453619..10a862674789 100644
--- a/drivers/staging/vboxvideo/vbox_main.c
+++ b/drivers/staging/vboxvideo/vbox_main.c
@@ -116,10 +116,10 @@ void vbox_framebuffer_dirty_rectangles(struct 
drm_framebuffer *fb,
struct vbva_cmd_hdr cmd_hdr;
unsigned int crtc_id = to_vbox_crtc(crtc)->crtc_id;
 
-   if ((rects[i].x1 > crtc->x + crtc->hwmode.hdisplay) ||
-   (rects[i].y1 > crtc->y + crtc->hwmode.vdisplay) ||
-   (rects[i].x2 < crtc->x) ||
-   (rects[i].y2 < crtc->y))
+   if (rects[i].x1 > crtc->x + crtc->hwmode.hdisplay ||
+   rects[i].y1 > crtc->y + crtc->hwmode.vdisplay ||
+   rects[i].x2 < crtc->x ||
+   rects[i].y2 < crtc->y)
continue;
 
cmd_hdr.x = (s16)rects[i].x1;
-- 
2.19.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 0/15] Add support for OLPC XO 1.75 Embedded Controller

2018-10-23 Thread Lubomir Rintel 
On Fri, 2018-10-19 at 16:57 +0300, Andy Shevchenko wrote:
> On Wed, Oct 10, 2018 at 8:23 PM Lubomir Rintel  wrote:
> > Hi.
> > 
> > This patchset adds support for the Embedded Controller on an OLPC XO
> > 1.75 machine. OLPC XO 1.75 is a MMP2 based ARM laptop. It plugs into
> > the existing OLPC platform infrastructure, currently used by the x86
> > based models.
> > 
> > The EC operates in SPI master mode, meaning the SOC is the SPI slave. It
> > uses extra handshake signal to signal readiness of SOC to accept data
> > from EC and to initiate a transaction if SOC wishes to submit data.
> > 
> > The SPI slave support for MMP2 was submitted separately:
> > https://lore.kernel.org/lkml/20181010170936.316862-1-lkund...@v3.sk/T/#t
> > 
> > THe "power: supply: olpc_battery: correct the temperature" patch was
> 
> The
> 
> > already sent out separately, but I'm including it because the last
> > commit of the set depends on it.
> > 
> > Tested to work on an OLPC XO 1.75 and also tested not to break x86
> > support with an OLPC XO 1 machine. I don't have a XO 1.5, but it's
> > unlikely this breaks it when XO 1 works.
> > 
> > Thanks in advance for reviews and feedback of any kind.
> 
> Thanks for the series.
> I'm about to review the patch 6, otherwise read my comments for the
> rest and consider addressing them.

Thank you very much for your feedback, it is very appreciated.

The XO 1.75 patch set has grown somewhat larger than I'm comfortable
with, so I need some time to digest and address the review. I hope I'll
be able to post a v2 some time after the 4.20 merge window closes and
respond to questions raised in individual patches before that.

> 
> > Lubo
> > 
> 
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/3] staging: greybus: loopback.c: do insertion in O(n) instead of O(n lg n)

2018-10-23 Thread Rasmus Villemoes 
On 2018-10-11 01:03, Bryan O'Donoghue wrote:
> On 05/10/2018 15:28, Rasmus Villemoes wrote:
>> Signed-off-by: Rasmus Villemoes
>> ---
>> I have no idea if the performance matters (it probably doesn't). Feel
>> free to ignore this and the followup cleanup.
> 
> What's the problem you're fixing here ?
> 
> Is it tested ?

I got curious why one would want to keep a linked list sorted in the
first place (it can't be for doing binary searches). But it seems that
gb_loopback_device::list is unused, along with the list_op_async. Given
that the below compiles, doesn't that prove that the code is
dead/unused, or what am I missing?

diff --git a/drivers/staging/greybus/loopback.c
b/drivers/staging/greybus/loopback.c
index 7080294f705c..e4d42c1dc284 100644
--- a/drivers/staging/greybus/loopback.c
+++ b/drivers/staging/greybus/loopback.c
@@ -47,8 +47,6 @@ struct gb_loopback_device {

/* We need to take a lock in atomic context */
spinlock_t lock;
-   struct list_head list;
-   struct list_head list_op_async;
wait_queue_head_t wq;
 };

@@ -68,7 +66,6 @@ struct gb_loopback {
struct kfifo kfifo_lat;
struct mutex mutex;
struct task_struct *task;
-   struct list_head entry;
struct device *dev;
wait_queue_head_t wq;
wait_queue_head_t wq_completion;
@@ -987,37 +984,6 @@ static const struct file_operations
gb_loopback_debugfs_latency_ops = {
.release= single_release,
 };

-static int gb_loopback_bus_id_compare(void *priv, struct list_head *lha,
- struct list_head *lhb)
-{
-   struct gb_loopback *a = list_entry(lha, struct gb_loopback, entry);
-   struct gb_loopback *b = list_entry(lhb, struct gb_loopback, entry);
-   struct gb_connection *ca = a->connection;
-   struct gb_connection *cb = b->connection;
-
-   if (ca->bundle->intf->interface_id < cb->bundle->intf->interface_id)
-   return -1;
-   if (cb->bundle->intf->interface_id < ca->bundle->intf->interface_id)
-   return 1;
-   if (ca->bundle->id < cb->bundle->id)
-   return -1;
-   if (cb->bundle->id < ca->bundle->id)
-   return 1;
-   if (ca->intf_cport_id < cb->intf_cport_id)
-   return -1;
-   else if (cb->intf_cport_id < ca->intf_cport_id)
-   return 1;
-
-   return 0;
-}
-
-static void gb_loopback_insert_id(struct gb_loopback *gb)
-{
-   /* perform an insertion sort */
-   list_add_tail(>entry, _dev.list);
-   list_sort(NULL, _dev.list, gb_loopback_bus_id_compare);
-}
-
 #define DEBUGFS_NAMELEN 32

 static int gb_loopback_probe(struct gb_bundle *bundle,
@@ -1113,7 +1079,6 @@ static int gb_loopback_probe(struct gb_bundle *bundle,
}

spin_lock_irqsave(_dev.lock, flags);
-   gb_loopback_insert_id(gb);
gb_dev.count++;
spin_unlock_irqrestore(_dev.lock, flags);

@@ -1169,7 +1134,6 @@ static void gb_loopback_disconnect(struct
gb_bundle *bundle)

spin_lock_irqsave(_dev.lock, flags);
gb_dev.count--;
-   list_del(>entry);
spin_unlock_irqrestore(_dev.lock, flags);

device_unregister(gb->dev);
@@ -1196,8 +1160,6 @@ static int loopback_init(void)
 {
int retval;

-   INIT_LIST_HEAD(_dev.list);
-   INIT_LIST_HEAD(_dev.list_op_async);
spin_lock_init(_dev.lock);
gb_dev.root = debugfs_create_dir("gb_loopback", NULL);

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] staging: iio: ad2s1210: Switch to the gpio descriptor interface

2018-10-23 Thread Slawomir Stepien 
On paź 22, 2018 22:09, Nishad Kamdar wrote:
> Use the gpiod interface instead of the deprecated old non-descriptor
> interface.

Hi

> Signed-off-by: Nishad Kamdar 
> ---
> Changes in v2:
>  - Use the spi_device struct embedded in st instead
>of passing it as an argument to ad2s1210_setup_gpios().
>  - Use an array of structs to reduce redundant code in
>in ad2s1210_setup_gpios().
>  - Remove ad2s1210_free_gpios() as devm API is being used.

Some few more comments on that changes below.

> ---
>  drivers/staging/iio/resolver/ad2s1210.c | 91 +
>  drivers/staging/iio/resolver/ad2s1210.h |  3 -
>  2 files changed, 48 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1210.c 
> b/drivers/staging/iio/resolver/ad2s1210.c
> index ac13b99bd9cb..e4c222b47574 100644
> --- a/drivers/staging/iio/resolver/ad2s1210.c
> +++ b/drivers/staging/iio/resolver/ad2s1210.c
> @@ -15,7 +15,7 @@
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 
>  #include 
>  
>  #include 
> @@ -69,10 +69,21 @@ enum ad2s1210_mode {
>  
>  static const unsigned int ad2s1210_resolution_value[] = { 10, 12, 14, 16 };
>  
> +struct ad2s1210_gpio {
> + struct gpio_desc *ptr;

You are going to use this ptr to change a pointer it points to. So it must be a
pointer to pointer: **ptr;

> + char *name;

I guess it should be const char *.

> + unsigned long flags;
> +};
> +
>  struct ad2s1210_state {
>   const struct ad2s1210_platform_data *pdata;
>   struct mutex lock;
>   struct spi_device *sdev;
> + struct gpio_desc *sample;
> + struct gpio_desc *a0;
> + struct gpio_desc *a1;
> + struct gpio_desc *res0;
> + struct gpio_desc *res1;
>   unsigned int fclkin;
>   unsigned int fexcit;
>   bool hysteresis;
> @@ -91,8 +102,8 @@ static const int ad2s1210_mode_vals[4][2] = {
>  static inline void ad2s1210_set_mode(enum ad2s1210_mode mode,
>struct ad2s1210_state *st)
>  {
> - gpio_set_value(st->pdata->a[0], ad2s1210_mode_vals[mode][0]);
> - gpio_set_value(st->pdata->a[1], ad2s1210_mode_vals[mode][1]);
> + gpiod_set_value(st->a0, ad2s1210_mode_vals[mode][0]);
> + gpiod_set_value(st->a1, ad2s1210_mode_vals[mode][1]);
>   st->mode = mode;
>  }
>  
> @@ -152,8 +163,8 @@ int ad2s1210_update_frequency_control_word(struct 
> ad2s1210_state *st)
>  
>  static unsigned char ad2s1210_read_resolution_pin(struct ad2s1210_state *st)
>  {
> - int resolution = (gpio_get_value(st->pdata->res[0]) << 1) |
> -   gpio_get_value(st->pdata->res[1]);
> + int resolution = (gpiod_get_value(st->res0) << 1) |
> +   gpiod_get_value(st->res1);
>  
>   return ad2s1210_resolution_value[resolution];
>  }
> @@ -164,10 +175,10 @@ static const int ad2s1210_res_pins[4][2] = {
>  
>  static inline void ad2s1210_set_resolution_pin(struct ad2s1210_state *st)
>  {
> - gpio_set_value(st->pdata->res[0],
> -ad2s1210_res_pins[(st->resolution - 10) / 2][0]);
> - gpio_set_value(st->pdata->res[1],
> -ad2s1210_res_pins[(st->resolution - 10) / 2][1]);
> + gpiod_set_value(st->res0,
> + ad2s1210_res_pins[(st->resolution - 10) / 2][0]);
> + gpiod_set_value(st->res1,
> + ad2s1210_res_pins[(st->resolution - 10) / 2][1]);
>  }
>  
>  static inline int ad2s1210_soft_reset(struct ad2s1210_state *st)
> @@ -401,15 +412,15 @@ static ssize_t ad2s1210_clear_fault(struct device *dev,
>   int ret;
>  
>   mutex_lock(>lock);
> - gpio_set_value(st->pdata->sample, 0);
> + gpiod_set_value(st->sample, 0);
>   /* delay (2 * tck + 20) nano seconds */
>   udelay(1);
> - gpio_set_value(st->pdata->sample, 1);
> + gpiod_set_value(st->sample, 1);
>   ret = ad2s1210_config_read(st, AD2S1210_REG_FAULT);
>   if (ret < 0)
>   goto error_ret;
> - gpio_set_value(st->pdata->sample, 0);
> - gpio_set_value(st->pdata->sample, 1);
> + gpiod_set_value(st->sample, 0);
> + gpiod_set_value(st->sample, 1);
>  error_ret:
>   mutex_unlock(>lock);
>  
> @@ -466,7 +477,7 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
>   s16 vel;
>  
>   mutex_lock(>lock);
> - gpio_set_value(st->pdata->sample, 0);
> + gpiod_set_value(st->sample, 0);
>   /* delay (6 * tck + 20) nano seconds */
>   udelay(1);
>  
> @@ -512,7 +523,7 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
>   }
>  
>  error_ret:
> - gpio_set_value(st->pdata->sample, 1);
> + gpiod_set_value(st->sample, 1);
>   /* delay (2 * tck + 20) nano seconds */
>   udelay(1);
>   mutex_unlock(>lock);
> @@ -630,30 +641,30 @@ static const struct iio_info ad2s1210_info = {
>  
>  static int ad2s1210_setup_gpios(struct ad2s1210_state *st)
>  {
> - unsigned long flags = st->pdata->gpioin ? GPIOF_DIR_IN : GPIOF_DIR_OUT;
> - struct gpio

RE: [PATCH] staging: android: ion: Fixed uninitialized heap name access

2018-10-23 Thread Skidanov, Alexey 



> -Original Message-
> From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> Sent: Tuesday, October 23, 2018 08:33
> To: Skidanov, Alexey 
> Cc: Laura Abbott ; gre...@linuxfoundation.org;
> de...@driverdev.osuosl.org
> Subject: Re: [PATCH] staging: android: ion: Fixed uninitialized heap name
> access
> 
> On Mon, Oct 22, 2018 at 05:47:08PM +0300, Alexey Skidanov wrote:
> >
> >
> > On 10/22/18 17:32, Laura Abbott wrote:
> > > On 10/22/2018 07:02 AM, Alexey Skidanov wrote:
> > >> The heap name might be uninitialized and access might crash the
> > >> kernel.
> > >>
> > >
> > > The heap name should never be null so this seems like this is being
> > > fixed in the wrong place. Can you explain more how you are hitting
> > > this issue?
> > Sure. Carve out heap name is uninitialized. There is the next patch
> > fixing it. But to be on the safe side, I have added the check.
> >
> 
> You keep saying uninitialized but you mean NULL.
I meant the uninitialized name, not the pointer. 
> 
> ion_carveout_heap_create() is never called so far as I can see so this
> isn't an issue in real life.  It feels like it would be detected right
ion_carveout_heap_create() is the only way to create this kind of heap. 
You are correct that currently it's never called - it's designed to be called 
by 
board specific code and in the meanwhile there is no standard way to do it. 
> away when that code was used.  We should just apply your follow on
> patch instead.
> 
> regards,
> dan carpenter
Thanks,
Alexey

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel