[PATCH 00/28] staging: most: fix issues

[PATCH 00/28] staging: most: fix issues

[Christian Gromm]

This patch set is needed to fix open issues of the MOST driver.

Christian Gromm (28):
  staging: most: allocate only all requested memory
  staging: most: dim2: remove clock speed processing from the HDM
  staging: most: i2c: prevent division by zero
  staging: most: i2c: remove unnecessary poison_channel call
  staging: most: add channel property dbr_size
  staging: most: aim-sound: add flexible format support
  staging: most: i2c: shorten lifetime of IRQ handler
  staging: most: i2c: do not wait in work function
  staging: most: i2c: avoid polling in case of misconfig
  staging: most: i2c: prevent zero delay polling
  staging: most: i2c: trace real polling rate
  staging: most: i2c: remove redundant is_open
  staging: most: i2c: remove redundant list_mutex
  staging: most: i2c: reduce parameters inconsistency
  staging: most: make interface drivers allocate coherent memory
  staging: most: sound: call snd_card_new with struct device
  staging: most: cdev: avoid warning about potentially uninitialized
    variable
  staging: most: cdev: fix chrdev_region leak
  staging: most: usb: add ep number to log
  staging: most: cdev: fix function return value
  staging: most: dim2: fix startup sequence
  staging: most: cdev: fix race condition
  staging: most: dim2: use device tree
  staging: most: dim2: read clock speed from the device
  staging: most: dim2: use device to allocate coherent memory
  staging: most: usb: don't set URB_ZERO_PACKET flag for synchronous
    data
  staging: most: usb: fix usb_disconnect race condition
  staging: most: usb: remove local variable

 drivers/staging/most/cdev/cdev.c   |  16 +-
 drivers/staging/most/core.c        |  68 +++++--
 drivers/staging/most/core.h        |   4 +
 drivers/staging/most/dim2/Kconfig  |   2 +-
 drivers/staging/most/dim2/dim2.c   | 397 +++++++++++++++++++++++++++----------
 drivers/staging/most/dim2/dim2.h   |  21 --
 drivers/staging/most/dim2/hal.c    |   9 +-
 drivers/staging/most/i2c/i2c.c     | 140 +++++--------
 drivers/staging/most/sound/sound.c | 123 ++++++------
 drivers/staging/most/usb/usb.c     |  55 +++--
 10 files changed, 519 insertions(+), 316 deletions(-)
 delete mode 100644 drivers/staging/most/dim2/dim2.h

-- 
2.7.4

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

[PATCH 01/28] staging: most: allocate only all requested memory

[Christian Gromm]

This prohibits the allocation of the memory for the MBOs if only the
part of the MBOs, requested by the application, may be allocated.  The
function arm_mbo_chain, if cannot allocate all requested MBO, frees all
prior allocated memory and returns 0.

Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/core.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/most/core.c b/drivers/staging/most/core.c
index 8f28335..965409e 100644
--- a/drivers/staging/most/core.c
+++ b/drivers/staging/most/core.c
@@ -952,18 +952,17 @@ static int arm_mbo_chain(struct most_channel *c, int dir,
 			 void (*compl)(struct mbo *))
 {
 	unsigned int i;
-	int retval;
 	struct mbo *mbo;
+	unsigned long flags;
 	u32 coherent_buf_size = c->cfg.buffer_size + c->cfg.extra_len;
 
 	atomic_set(&c->mbo_nq_level, 0);
 
 	for (i = 0; i < c->cfg.num_buffers; i++) {
 		mbo = kzalloc(sizeof(*mbo), GFP_KERNEL);
-		if (!mbo) {
-			retval = i;
-			goto _exit;
-		}
+		if (!mbo)
+			goto flush_fifos;
+
 		mbo->context = c;
 		mbo->ifp = c->iface;
 		mbo->hdm_channel_id = c->channel_id;
@@ -971,26 +970,28 @@ static int arm_mbo_chain(struct most_channel *c, int dir,
 						       coherent_buf_size,
 						       &mbo->bus_address,
 						       GFP_KERNEL);
-		if (!mbo->virt_address) {
-			pr_info("WARN: No DMA coherent buffer.\n");
-			retval = i;
-			goto _error1;
-		}
+		if (!mbo->virt_address)
+			goto release_mbo;
+
 		mbo->complete = compl;
 		mbo->num_buffers_ptr = &dummy_num_buffers;
 		if (dir == MOST_CH_RX) {
 			nq_hdm_mbo(mbo);
 			atomic_inc(&c->mbo_nq_level);
 		} else {
-			arm_mbo(mbo);
+			spin_lock_irqsave(&c->fifo_lock, flags);
+			list_add_tail(&mbo->list, &c->fifo);
+			spin_unlock_irqrestore(&c->fifo_lock, flags);
 		}
 	}
-	return i;
+	return c->cfg.num_buffers;
 
-_error1:
+release_mbo:
 	kfree(mbo);
-_exit:
-	return retval;
+
+flush_fifos:
+	flush_channel_fifos(c);
+	return 0;
 }
 
 /**
-- 
2.7.4

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

Re: [PATCH 01/28] staging: most: allocate only all requested memory

[Dan Carpenter]

On Tue, May 08, 2018 at 11:44:49AM +0200, Christian Gromm wrote:
> This prohibits the allocation of the memory for the MBOs if only the
> part of the MBOs, requested by the application, may be allocated.  The
> function arm_mbo_chain, if cannot allocate all requested MBO, frees all
> prior allocated memory and returns 0.
> 
> Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
> Signed-off-by: Christian Gromm <christian.gromm@microchip.com>

I don't understand how Andrey is involved with this code.  Did he write
it?  Why isn't he on the CC list?

It's possible that you have explained this before, but I have the memory
of a gnat.  #TabulaRasa

regards,
dan carpenter

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

Re: [PATCH 01/28] staging: most: allocate only all requested memory

[Christian Gromm]

On 09.05.2018 15:19, Dan Carpenter wrote:
> On Tue, May 08, 2018 at 11:44:49AM +0200, Christian Gromm wrote:
>> This prohibits the allocation of the memory for the MBOs if only the
>> part of the MBOs, requested by the application, may be allocated.  The
>> function arm_mbo_chain, if cannot allocate all requested MBO, frees all
>> prior allocated memory and returns 0.
>>
>> Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
>> Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> 
> I don't understand how Andrey is involved with this code.  Did he write
> it?  Why isn't he on the CC list?

Andrey and I are writing the code for this driver and our internal dev
process on Gitlab is based on reviews.
Which means that everything I push needs to be reviewed by Andrey and
vice versa before it gets merged to master. Then I have to do the
upstream work.

I didn't put him on CC explicitly, because git-sendemail does it,
doesn't it?

thanks,
Chris

> 
> It's possible that you have explained this before, but I have the memory
> of a gnat.  #TabulaRasa
> 
> regards,
> dan carpenter
> 

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

Re: [PATCH 01/28] staging: most: allocate only all requested memory

[Dan Carpenter]

On Wed, May 09, 2018 at 04:09:21PM +0200, Christian Gromm wrote:
> On 09.05.2018 15:19, Dan Carpenter wrote:
> > On Tue, May 08, 2018 at 11:44:49AM +0200, Christian Gromm wrote:
> > > This prohibits the allocation of the memory for the MBOs if only the
> > > part of the MBOs, requested by the application, may be allocated.  The
> > > function arm_mbo_chain, if cannot allocate all requested MBO, frees all
> > > prior allocated memory and returns 0.
> > > 
> > > Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
> > > Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> > 
> > I don't understand how Andrey is involved with this code.  Did he write
> > it?  Why isn't he on the CC list?
> 
> Andrey and I are writing the code for this driver and our internal dev
> process on Gitlab is based on reviews.
> Which means that everything I push needs to be reviewed by Andrey and
> vice versa before it gets merged to master. Then I have to do the
> upstream work.
> 
> I didn't put him on CC explicitly, because git-sendemail does it,
> doesn't it?

It didn't CC him so far as I can see...  BCC?

Signed-of-by basically means you handled the code but didn't secretly
insert any of SCO's top secret UNIX code into the patch.  It's sort of
a legal thing.

It sort of sounds like Andrey is reviewing your code?  In that case,
probably Reviewed-by is the correct tag.

Normally the tags are sort of in chronoligical order.

Reported-by: Sally
Signed-off-by: Bob
Reviewed-by: Mary
Signed-off-by: Subsystem Maintainer

So it always confuses me when the first person to Sign off isn't the
author.

regards,
dan carpenter
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 01/28] staging: most: allocate only all requested memory

[Christian Gromm]

On 09.05.2018 16:54, Dan Carpenter wrote:
> On Wed, May 09, 2018 at 04:09:21PM +0200, Christian Gromm wrote:
>> On 09.05.2018 15:19, Dan Carpenter wrote:
>>> On Tue, May 08, 2018 at 11:44:49AM +0200, Christian Gromm wrote:
>>>> This prohibits the allocation of the memory for the MBOs if only the
>>>> part of the MBOs, requested by the application, may be allocated.  The
>>>> function arm_mbo_chain, if cannot allocate all requested MBO, frees all
>>>> prior allocated memory and returns 0.
>>>>
>>>> Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
>>>> Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
>>>
>>> I don't understand how Andrey is involved with this code.  Did he write
>>> it?  Why isn't he on the CC list?
>>
>> Andrey and I are writing the code for this driver and our internal dev
>> process on Gitlab is based on reviews.
>> Which means that everything I push needs to be reviewed by Andrey and
>> vice versa before it gets merged to master. Then I have to do the
>> upstream work.
>>
>> I didn't put him on CC explicitly, because git-sendemail does it,
>> doesn't it?
> 
> It didn't CC him so far as I can see...  BCC?

Hmm, I'll make sure that he'll be put on CC next time.

> 
> Signed-of-by basically means you handled the code but didn't secretly
> insert any of SCO's top secret UNIX code into the patch.  It's sort of
> a legal thing.

To be honest, I don't pay much attention to the order of those tags.

> 
> It sort of sounds like Andrey is reviewing your code?  In that case,
> probably Reviewed-by is the correct tag.

Yes. We are basically sitting in the same building and talk things
through, before merging them. I'll suggest your proposal.

thanks,
Chris

> 
> Normally the tags are sort of in chronoligical order.
> 
> Reported-by: Sally
> Signed-off-by: Bob
> Reviewed-by: Mary
> Signed-off-by: Subsystem Maintainer
> 
> So it always confuses me when the first person to Sign off isn't the
> author.
> 
> regards,
> dan carpenter
> 

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

Re: [PATCH 01/28] staging: most: allocate only all requested memory

[Greg KH]

On Wed, May 09, 2018 at 05:18:47PM +0200, Christian Gromm wrote:
> On 09.05.2018 16:54, Dan Carpenter wrote:
> > On Wed, May 09, 2018 at 04:09:21PM +0200, Christian Gromm wrote:
> > > On 09.05.2018 15:19, Dan Carpenter wrote:
> > > > On Tue, May 08, 2018 at 11:44:49AM +0200, Christian Gromm wrote:
> > > > > This prohibits the allocation of the memory for the MBOs if only the
> > > > > part of the MBOs, requested by the application, may be allocated.  The
> > > > > function arm_mbo_chain, if cannot allocate all requested MBO, frees all
> > > > > prior allocated memory and returns 0.
> > > > > 
> > > > > Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
> > > > > Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> > > > 
> > > > I don't understand how Andrey is involved with this code.  Did he write
> > > > it?  Why isn't he on the CC list?
> > > 
> > > Andrey and I are writing the code for this driver and our internal dev
> > > process on Gitlab is based on reviews.
> > > Which means that everything I push needs to be reviewed by Andrey and
> > > vice versa before it gets merged to master. Then I have to do the
> > > upstream work.
> > > 
> > > I didn't put him on CC explicitly, because git-sendemail does it,
> > > doesn't it?
> > 
> > It didn't CC him so far as I can see...  BCC?
> 
> Hmm, I'll make sure that he'll be put on CC next time.
> 
> > 
> > Signed-of-by basically means you handled the code but didn't secretly
> > insert any of SCO's top secret UNIX code into the patch.  It's sort of
> > a legal thing.
> 
> To be honest, I don't pay much attention to the order of those tags.
> 
> > 
> > It sort of sounds like Andrey is reviewing your code?  In that case,
> > probably Reviewed-by is the correct tag.
> 
> Yes. We are basically sitting in the same building and talk things
> through, before merging them. I'll suggest your proposal.

There is also Co-Developed-by: for stuff like this if needed.

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 01/28] staging: most: allocate only all requested memory

[Christian Gromm]

On 09.05.2018 17:29, Greg KH wrote:
> On Wed, May 09, 2018 at 05:18:47PM +0200, Christian Gromm wrote:
>> On 09.05.2018 16:54, Dan Carpenter wrote:
>>> On Wed, May 09, 2018 at 04:09:21PM +0200, Christian Gromm wrote:
>>>> On 09.05.2018 15:19, Dan Carpenter wrote:
>>>>> On Tue, May 08, 2018 at 11:44:49AM +0200, Christian Gromm wrote:
>>>>>> This prohibits the allocation of the memory for the MBOs if only the
>>>>>> part of the MBOs, requested by the application, may be allocated.  The
>>>>>> function arm_mbo_chain, if cannot allocate all requested MBO, frees all
>>>>>> prior allocated memory and returns 0.
>>>>>>
>>>>>> Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
>>>>>> Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
>>>>>
>>>>> I don't understand how Andrey is involved with this code.  Did he write
>>>>> it?  Why isn't he on the CC list?
>>>>
>>>> Andrey and I are writing the code for this driver and our internal dev
>>>> process on Gitlab is based on reviews.
>>>> Which means that everything I push needs to be reviewed by Andrey and
>>>> vice versa before it gets merged to master. Then I have to do the
>>>> upstream work.
>>>>
>>>> I didn't put him on CC explicitly, because git-sendemail does it,
>>>> doesn't it?
>>>
>>> It didn't CC him so far as I can see...  BCC?
>>
>> Hmm, I'll make sure that he'll be put on CC next time.
>>
>>>
>>> Signed-of-by basically means you handled the code but didn't secretly
>>> insert any of SCO's top secret UNIX code into the patch.  It's sort of
>>> a legal thing.
>>
>> To be honest, I don't pay much attention to the order of those tags.
>>
>>>
>>> It sort of sounds like Andrey is reviewing your code?  In that case,
>>> probably Reviewed-by is the correct tag.
>>
>> Yes. We are basically sitting in the same building and talk things
>> through, before merging them. I'll suggest your proposal.
> 
> There is also Co-Developed-by: for stuff like this if needed.

I'll keep that in mind, too.

thanks,
Chris

> 
> thanks,
> 
> greg k-h
> 

[PATCH 02/28] staging: most: dim2: remove clock speed processing from the HDM

[Christian Gromm]

This removes the module parameter clock_speed from the HDM code.

Instead, the platform-dependent clock speed must be delivered by the
platform driver with the help of the dim2_platform_data.clk_speed.

Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/dim2/dim2.c | 50 +++++++---------------------------------
 drivers/staging/most/dim2/dim2.h |  7 +++---
 2 files changed, 12 insertions(+), 45 deletions(-)

diff --git a/drivers/staging/most/dim2/dim2.c b/drivers/staging/most/dim2/dim2.c
index f9bc7de..fa4559b 100644
--- a/drivers/staging/most/dim2/dim2.c
+++ b/drivers/staging/most/dim2/dim2.c
@@ -32,11 +32,6 @@
 #define MAX_BUF_SIZE_PACKET     2048
 #define MAX_BUF_SIZE_STREAMING  (8 * 1024)
 
-/* command line parameter to select clock speed */
-static char *clock_speed;
-module_param(clock_speed, charp, 0000);
-MODULE_PARM_DESC(clock_speed, "MediaLB Clock Speed");
-
 /*
  * The parameter representing the number of frames per sub-buffer for
  * synchronous channels.  Valid values: [0 .. 6].
@@ -78,7 +73,6 @@ struct hdm_channel {
  * @most_iface: most interface structure
  * @capabilities: an array of channel capability data
  * @io_base: I/O register base address
- * @clk_speed: user selectable (through command line parameter) clock speed
  * @netinfo_task: thread to deliver network status
  * @netinfo_waitq: waitq for the thread to sleep
  * @deliver_netinfo: to identify whether network status received
@@ -93,7 +87,6 @@ struct dim2_hdm {
 	struct most_interface most_iface;
 	char name[16 + sizeof "dim2-"];
 	void __iomem *io_base;
-	int clk_speed;
 	struct task_struct *netinfo_task;
 	wait_queue_head_t netinfo_waitq;
 	int deliver_netinfo;
@@ -158,52 +151,25 @@ void dimcb_on_error(u8 error_id, const char *error_message)
 /**
  * startup_dim - initialize the dim2 interface
  * @pdev: platform device
- *
- * Get the value of command line parameter "clock_speed" if given or use the
- * default value, enable the clock and PLL, and initialize the dim2 interface.
  */
 static int startup_dim(struct platform_device *pdev)
 {
 	struct dim2_hdm *dev = platform_get_drvdata(pdev);
 	struct dim2_platform_data *pdata = pdev->dev.platform_data;
 	u8 hal_ret;
+	int ret;
 
-	dev->clk_speed = -1;
-
-	if (clock_speed) {
-		if (!strcmp(clock_speed, "256fs"))
-			dev->clk_speed = CLK_256FS;
-		else if (!strcmp(clock_speed, "512fs"))
-			dev->clk_speed = CLK_512FS;
-		else if (!strcmp(clock_speed, "1024fs"))
-			dev->clk_speed = CLK_1024FS;
-		else if (!strcmp(clock_speed, "2048fs"))
-			dev->clk_speed = CLK_2048FS;
-		else if (!strcmp(clock_speed, "3072fs"))
-			dev->clk_speed = CLK_3072FS;
-		else if (!strcmp(clock_speed, "4096fs"))
-			dev->clk_speed = CLK_4096FS;
-		else if (!strcmp(clock_speed, "6144fs"))
-			dev->clk_speed = CLK_6144FS;
-		else if (!strcmp(clock_speed, "8192fs"))
-			dev->clk_speed = CLK_8192FS;
-	}
-
-	if (dev->clk_speed == -1) {
-		pr_info("Bad or missing clock speed parameter, using default value: 3072fs\n");
-		dev->clk_speed = CLK_3072FS;
-	} else {
-		pr_info("Selected clock speed: %s\n", clock_speed);
+	if (!pdata) {
+		pr_err("missing platform data\n");
+		return -EINVAL;
 	}
-	if (pdata && pdata->init) {
-		int ret = pdata->init(pdata, dev->io_base, dev->clk_speed);
 
-		if (ret)
-			return ret;
-	}
+	ret = pdata->init ? pdata->init(pdata, dev->io_base) : 0;
+	if (ret)
+		return ret;
 
 	pr_info("sync: num of frames per sub-buffer: %u\n", fcnt);
-	hal_ret = dim_startup(dev->io_base, dev->clk_speed, fcnt);
+	hal_ret = dim_startup(dev->io_base, pdata->clk_speed, fcnt);
 	if (hal_ret != DIM_NO_ERROR) {
 		pr_err("dim_startup failed: %d\n", hal_ret);
 		if (pdata && pdata->destroy)
diff --git a/drivers/staging/most/dim2/dim2.h b/drivers/staging/most/dim2/dim2.h
index 6a9fc51..999d4d4 100644
--- a/drivers/staging/most/dim2/dim2.h
+++ b/drivers/staging/most/dim2/dim2.h
@@ -8,14 +8,15 @@
 #ifndef DIM2_HDM_H
 #define	DIM2_HDM_H
 
+#include <linux/types.h>
+
 struct device;
 
 /* platform dependent data for dim2 interface */
 struct dim2_platform_data {
-	int (*init)(struct dim2_platform_data *pd, void __iomem *io_base,
-		    int clk_speed);
+	int (*init)(struct dim2_platform_data *pd, void __iomem *io_base);
 	void (*destroy)(struct dim2_platform_data *pd);
-	void *priv;
+	u8 clk_speed;
 };
 
 #endif	/* DIM2_HDM_H */
-- 
2.7.4

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

[PATCH 03/28] staging: most: i2c: prevent division by zero

[Christian Gromm]

This prevents division by zero scan_rate.

The zero scan_rate does not need any special action as it actually means
"never poll again".

Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/i2c/i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/most/i2c/i2c.c b/drivers/staging/most/i2c/i2c.c
index 141239f..501eec0 100644
--- a/drivers/staging/most/i2c/i2c.c
+++ b/drivers/staging/most/i2c/i2c.c
@@ -252,7 +252,7 @@ static void pending_rx_work(struct work_struct *work)
 	do_rx_work(dev);
 
 	if (dev->polling_mode) {
-		if (dev->is_open[CH_RX])
+		if (dev->is_open[CH_RX] && scan_rate)
 			schedule_delayed_work(&dev->rx.dwork,
 					      msecs_to_jiffies(MSEC_PER_SEC
 							       / scan_rate));
-- 
2.7.4

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

[PATCH 04/28] staging: most: i2c: remove unnecessary poison_channel call

[Christian Gromm]

This removes call of the poison_channel that is:
  - not allowed after most_deregister_interface;
  - is made during the most_deregister_interface call.

Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/i2c/i2c.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/staging/most/i2c/i2c.c b/drivers/staging/most/i2c/i2c.c
index 501eec0..8ec660b 100644
--- a/drivers/staging/most/i2c/i2c.c
+++ b/drivers/staging/most/i2c/i2c.c
@@ -376,16 +376,11 @@ static int i2c_probe(struct i2c_client *client, const struct i2c_device_id *id)
 static int i2c_remove(struct i2c_client *client)
 {
 	struct hdm_i2c *dev = i2c_get_clientdata(client);
-	int i;
 
 	if (!dev->polling_mode)
 		free_irq(client->irq, dev);
 
 	most_deregister_interface(&dev->most_iface);
-
-	for (i = 0 ; i < NUM_CHANNELS; i++)
-		if (dev->is_open[i])
-			poison_channel(&dev->most_iface, i);
 	cancel_delayed_work_sync(&dev->rx.dwork);
 	kfree(dev);
 
-- 
2.7.4

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

[PATCH 05/28] staging: most: add channel property dbr_size

[Christian Gromm]

This patch adds the channel property dbr_size to control the corresponding
buffer size of the channels of the DIM2 interface.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/core.c      | 22 ++++++++++++++++++++++
 drivers/staging/most/core.h      |  1 +
 drivers/staging/most/dim2/dim2.c | 10 ++++++++++
 drivers/staging/most/dim2/hal.c  |  9 ++++++---
 4 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/most/core.c b/drivers/staging/most/core.c
index 965409e..db3c7e2 100644
--- a/drivers/staging/most/core.c
+++ b/drivers/staging/most/core.c
@@ -420,6 +420,26 @@ static ssize_t set_packets_per_xact_store(struct device *dev,
 	return count;
 }
 
+static ssize_t set_dbr_size_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct most_channel *c = to_channel(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", c->cfg.dbr_size);
+}
+
+static ssize_t set_dbr_size_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t count)
+{
+	struct most_channel *c = to_channel(dev);
+	int ret = kstrtou16(buf, 0, &c->cfg.dbr_size);
+
+	if (ret)
+		return ret;
+	return count;
+}
+
 #define DEV_ATTR(_name)  (&dev_attr_##_name.attr)
 
 static DEVICE_ATTR_RO(available_directions);
@@ -435,6 +455,7 @@ static DEVICE_ATTR_RW(set_direction);
 static DEVICE_ATTR_RW(set_datatype);
 static DEVICE_ATTR_RW(set_subbuffer_size);
 static DEVICE_ATTR_RW(set_packets_per_xact);
+static DEVICE_ATTR_RW(set_dbr_size);
 
 static struct attribute *channel_attrs[] = {
 	DEV_ATTR(available_directions),
@@ -450,6 +471,7 @@ static struct attribute *channel_attrs[] = {
 	DEV_ATTR(set_datatype),
 	DEV_ATTR(set_subbuffer_size),
 	DEV_ATTR(set_packets_per_xact),
+	DEV_ATTR(set_dbr_size),
 	NULL,
 };
 
diff --git a/drivers/staging/most/core.h b/drivers/staging/most/core.h
index 884bd71..5b184e6 100644
--- a/drivers/staging/most/core.h
+++ b/drivers/staging/most/core.h
@@ -128,6 +128,7 @@ struct most_channel_config {
 	u16 extra_len;
 	u16 subbuffer_size;
 	u16 packets_per_xact;
+	u16 dbr_size;
 };
 
 /*
diff --git a/drivers/staging/most/dim2/dim2.c b/drivers/staging/most/dim2/dim2.c
index fa4559b..d15867a 100644
--- a/drivers/staging/most/dim2/dim2.c
+++ b/drivers/staging/most/dim2/dim2.c
@@ -61,6 +61,7 @@ struct hdm_channel {
 	char name[sizeof "caNNN"];
 	bool is_initialized;
 	struct dim_channel ch;
+	u16 *reset_dbr_size;
 	struct list_head pending_list;	/* before dim_enqueue_buffer() */
 	struct list_head started_list;	/* after dim_enqueue_buffer() */
 	enum most_channel_direction direction;
@@ -494,6 +495,12 @@ static int configure_channel(struct most_interface *most_iface, int ch_idx,
 	if (hdm_ch->is_initialized)
 		return -EPERM;
 
+	/* do not reset if the property was set by user, see poison_channel */
+	hdm_ch->reset_dbr_size = ccfg->dbr_size ? NULL : &ccfg->dbr_size;
+
+	/* zero value is default dbr_size, see dim2 hal */
+	hdm_ch->ch.dbr_size = ccfg->dbr_size;
+
 	switch (ccfg->data_type) {
 	case MOST_CH_CONTROL:
 		new_size = dim_norm_ctrl_async_buffer_size(buf_size);
@@ -574,6 +581,7 @@ static int configure_channel(struct most_interface *most_iface, int ch_idx,
 		dev->atx_idx = ch_idx;
 
 	spin_unlock_irqrestore(&dim_lock, flags);
+	ccfg->dbr_size = hdm_ch->ch.dbr_size;
 
 	return 0;
 }
@@ -689,6 +697,8 @@ static int poison_channel(struct most_interface *most_iface, int ch_idx)
 
 	complete_all_mbos(&hdm_ch->started_list);
 	complete_all_mbos(&hdm_ch->pending_list);
+	if (hdm_ch->reset_dbr_size)
+		*hdm_ch->reset_dbr_size = 0;
 
 	return ret;
 }
diff --git a/drivers/staging/most/dim2/hal.c b/drivers/staging/most/dim2/hal.c
index 17c04e1..699e02f 100644
--- a/drivers/staging/most/dim2/hal.c
+++ b/drivers/staging/most/dim2/hal.c
@@ -760,7 +760,8 @@ static u8 init_ctrl_async(struct dim_channel *ch, u8 type, u8 is_tx,
 	if (!check_channel_address(ch_address))
 		return DIM_INIT_ERR_CHANNEL_ADDRESS;
 
-	ch->dbr_size = ROUND_UP_TO(hw_buffer_size, DBR_BLOCK_SIZE);
+	if (!ch->dbr_size)
+		ch->dbr_size = ROUND_UP_TO(hw_buffer_size, DBR_BLOCK_SIZE);
 	ch->dbr_addr = alloc_dbr(ch->dbr_size);
 	if (ch->dbr_addr >= DBR_SIZE)
 		return DIM_INIT_ERR_OUT_OF_MEMORY;
@@ -846,7 +847,8 @@ u8 dim_init_isoc(struct dim_channel *ch, u8 is_tx, u16 ch_address,
 	if (!check_packet_length(packet_length))
 		return DIM_ERR_BAD_CONFIG;
 
-	ch->dbr_size = packet_length * ISOC_DBR_FACTOR;
+	if (!ch->dbr_size)
+		ch->dbr_size = packet_length * ISOC_DBR_FACTOR;
 	ch->dbr_addr = alloc_dbr(ch->dbr_size);
 	if (ch->dbr_addr >= DBR_SIZE)
 		return DIM_INIT_ERR_OUT_OF_MEMORY;
@@ -873,7 +875,8 @@ u8 dim_init_sync(struct dim_channel *ch, u8 is_tx, u16 ch_address,
 	if (!check_bytes_per_frame(bytes_per_frame))
 		return DIM_ERR_BAD_CONFIG;
 
-	ch->dbr_size = bytes_per_frame << bd_factor;
+	if (!ch->dbr_size)
+		ch->dbr_size = bytes_per_frame << bd_factor;
 	ch->dbr_addr = alloc_dbr(ch->dbr_size);
 	if (ch->dbr_addr >= DBR_SIZE)
 		return DIM_INIT_ERR_OUT_OF_MEMORY;
-- 
2.7.4

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

[PATCH 06/28] staging: most: aim-sound: add flexible format support

[Christian Gromm]

Currently, the only supported PCM formats are 1x8", "2x16", "2x24",
"2x32" or "6x16".

This adds support for the format "Nx{8,16,24,32}" that also includes the
exotic PCM formats like "4x16", "5x8", etc.

Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/sound/sound.c | 121 +++++++++++++++++++------------------
 1 file changed, 62 insertions(+), 59 deletions(-)

diff --git a/drivers/staging/most/sound/sound.c b/drivers/staging/most/sound/sound.c
index 83cec21..18f7224 100644
--- a/drivers/staging/most/sound/sound.c
+++ b/drivers/staging/most/sound/sound.c
@@ -460,21 +460,68 @@ static const struct snd_pcm_ops pcm_ops = {
 	.mmap       = snd_pcm_lib_mmap_vmalloc,
 };
 
-static int split_arg_list(char *buf, char **card_name, char **pcm_format)
+static int split_arg_list(char *buf, char **card_name, u16 *ch_num,
+			  char **sample_res)
 {
+	char *num;
+	int ret;
+
 	*card_name = strsep(&buf, ".");
-	if (!*card_name)
-		return -EIO;
-	*pcm_format = strsep(&buf, ".\n");
-	if (!*pcm_format)
+	if (!*card_name) {
+		pr_err("Missing sound card name\n");
 		return -EIO;
+	}
+	num = strsep(&buf, "x");
+	if (!num)
+		goto err;
+	ret = kstrtou16(num, 0, ch_num);
+	if (ret)
+		goto err;
+	*sample_res = strsep(&buf, ".\n");
+	if (!*sample_res)
+		goto err;
 	return 0;
+
+err:
+	pr_err("Bad PCM format\n");
+	return -EIO;
 }
 
+static const struct sample_resolution_info {
+	const char *sample_res;
+	int bytes;
+	u64 formats;
+} sinfo[] = {
+	{ "8", 1, SNDRV_PCM_FMTBIT_S8 },
+	{ "16", 2, SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE },
+	{ "24", 3, SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S24_3BE },
+	{ "32", 4, SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S32_BE },
+};
+
 static int audio_set_hw_params(struct snd_pcm_hardware *pcm_hw,
-			       char *pcm_format,
+			       u16 ch_num, char *sample_res,
 			       struct most_channel_config *cfg)
 {
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(sinfo); i++) {
+		if (!strcmp(sample_res, sinfo[i].sample_res))
+			goto found;
+	}
+	pr_err("Unsupported PCM format\n");
+	return -EIO;
+
+found:
+	if (!ch_num) {
+		pr_err("Bad number of channels\n");
+		return -EINVAL;
+	}
+
+	if (cfg->subbuffer_size != ch_num * sinfo[i].bytes) {
+		pr_err("Audio resolution doesn't fit subbuffer size\n");
+		return -EINVAL;
+	}
+
 	pcm_hw->info = MOST_PCM_INFO;
 	pcm_hw->rates = SNDRV_PCM_RATE_48000;
 	pcm_hw->rate_min = 48000;
@@ -484,54 +531,10 @@ static int audio_set_hw_params(struct snd_pcm_hardware *pcm_hw,
 	pcm_hw->period_bytes_max = cfg->buffer_size;
 	pcm_hw->periods_min = 1;
 	pcm_hw->periods_max = cfg->num_buffers;
-
-	if (!strcmp(pcm_format, "1x8")) {
-		if (cfg->subbuffer_size != 1)
-			goto error;
-		pr_info("PCM format is 8-bit mono\n");
-		pcm_hw->channels_min = 1;
-		pcm_hw->channels_max = 1;
-		pcm_hw->formats = SNDRV_PCM_FMTBIT_S8;
-	} else if (!strcmp(pcm_format, "2x16")) {
-		if (cfg->subbuffer_size != 4)
-			goto error;
-		pr_info("PCM format is 16-bit stereo\n");
-		pcm_hw->channels_min = 2;
-		pcm_hw->channels_max = 2;
-		pcm_hw->formats = SNDRV_PCM_FMTBIT_S16_LE |
-				  SNDRV_PCM_FMTBIT_S16_BE;
-	} else if (!strcmp(pcm_format, "2x24")) {
-		if (cfg->subbuffer_size != 6)
-			goto error;
-		pr_info("PCM format is 24-bit stereo\n");
-		pcm_hw->channels_min = 2;
-		pcm_hw->channels_max = 2;
-		pcm_hw->formats = SNDRV_PCM_FMTBIT_S24_3LE |
-				  SNDRV_PCM_FMTBIT_S24_3BE;
-	} else if (!strcmp(pcm_format, "2x32")) {
-		if (cfg->subbuffer_size != 8)
-			goto error;
-		pr_info("PCM format is 32-bit stereo\n");
-		pcm_hw->channels_min = 2;
-		pcm_hw->channels_max = 2;
-		pcm_hw->formats = SNDRV_PCM_FMTBIT_S32_LE |
-				  SNDRV_PCM_FMTBIT_S32_BE;
-	} else if (!strcmp(pcm_format, "6x16")) {
-		if (cfg->subbuffer_size != 12)
-			goto error;
-		pr_info("PCM format is 16-bit 5.1 multi channel\n");
-		pcm_hw->channels_min = 6;
-		pcm_hw->channels_max = 6;
-		pcm_hw->formats = SNDRV_PCM_FMTBIT_S16_LE |
-				  SNDRV_PCM_FMTBIT_S16_BE;
-	} else {
-		pr_err("PCM format %s not supported\n", pcm_format);
-		return -EIO;
-	}
+	pcm_hw->channels_min = ch_num;
+	pcm_hw->channels_max = ch_num;
+	pcm_hw->formats = sinfo[i].formats;
 	return 0;
-error:
-	pr_err("Audio resolution doesn't fit subbuffer size\n");
-	return -EINVAL;
 }
 
 /**
@@ -558,7 +561,8 @@ static int audio_probe_channel(struct most_interface *iface, int channel_id,
 	int ret;
 	int direction;
 	char *card_name;
-	char *pcm_format;
+	u16 ch_num;
+	char *sample_res;
 
 	if (!iface)
 		return -EINVAL;
@@ -582,11 +586,9 @@ static int audio_probe_channel(struct most_interface *iface, int channel_id,
 		direction = SNDRV_PCM_STREAM_CAPTURE;
 	}
 
-	ret = split_arg_list(arg_list, &card_name, &pcm_format);
-	if (ret < 0) {
-		pr_info("PCM format missing\n");
+	ret = split_arg_list(arg_list, &card_name, &ch_num, &sample_res);
+	if (ret < 0)
 		return ret;
-	}
 
 	ret = snd_card_new(NULL, -1, card_name, THIS_MODULE,
 			   sizeof(*channel), &card);
@@ -600,7 +602,8 @@ static int audio_probe_channel(struct most_interface *iface, int channel_id,
 	channel->id = channel_id;
 	init_waitqueue_head(&channel->playback_waitq);
 
-	ret = audio_set_hw_params(&channel->pcm_hardware, pcm_format, cfg);
+	ret = audio_set_hw_params(&channel->pcm_hardware, ch_num, sample_res,
+				  cfg);
 	if (ret)
 		goto err_free_card;
 
-- 
2.7.4

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

[PATCH 07/28] staging: most: i2c: shorten lifetime of IRQ handler

[Christian Gromm]

Currently the IRQ handler used for the rx channel lives between the
functions i2c_probe and i2c_remove. This patch shortens the lifetime
and keeps the handler alive only between the functions configure_channel
and poison_channel.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/i2c/i2c.c | 40 +++++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/most/i2c/i2c.c b/drivers/staging/most/i2c/i2c.c
index 8ec660b..b382f09 100644
--- a/drivers/staging/most/i2c/i2c.c
+++ b/drivers/staging/most/i2c/i2c.c
@@ -56,6 +56,8 @@ struct hdm_i2c {
 
 #define to_hdm(iface) container_of(iface, struct hdm_i2c, most_iface)
 
+static irqreturn_t most_irq_handler(int, void *);
+
 /**
  * configure_channel - called from MOST core to configure a channel
  * @iface: interface the channel belongs to
@@ -71,6 +73,7 @@ static int configure_channel(struct most_interface *most_iface,
 			     int ch_idx,
 			     struct most_channel_config *channel_config)
 {
+	int ret;
 	struct hdm_i2c *dev = to_hdm(most_iface);
 
 	BUG_ON(ch_idx < 0 || ch_idx >= NUM_CHANNELS);
@@ -86,7 +89,21 @@ static int configure_channel(struct most_interface *most_iface,
 		return -EPERM;
 	}
 
+	if (channel_config->direction == MOST_CH_RX) {
+		dev->polling_mode = polling_req || dev->client->irq <= 0;
+		if (!dev->polling_mode) {
+			pr_info("Requesting IRQ: %d\n", dev->client->irq);
+			ret = request_irq(dev->client->irq, most_irq_handler, 0,
+					  dev->client->name, dev);
+			if (ret) {
+				pr_info("IRQ request failed: %d, falling back to polling\n",
+					ret);
+				dev->polling_mode = true;
+			}
+		}
+	}
 	if ((channel_config->direction == MOST_CH_RX) && (dev->polling_mode)) {
+		pr_info("Using polling at rate: %d times/sec\n", scan_rate);
 		schedule_delayed_work(&dev->rx.dwork,
 				      msecs_to_jiffies(MSEC_PER_SEC / 4));
 	}
@@ -160,6 +177,10 @@ static int poison_channel(struct most_interface *most_iface,
 	dev->is_open[ch_idx] = false;
 
 	if (ch_idx == CH_RX) {
+		if (!dev->polling_mode)
+			free_irq(dev->client->irq, dev);
+		cancel_delayed_work_sync(&dev->rx.dwork);
+
 		mutex_lock(&dev->rx.list_mutex);
 		while (!list_empty(&dev->rx.list)) {
 			mbo = list_first_mbo(&dev->rx.list);
@@ -347,21 +368,6 @@ static int i2c_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		return ret;
 	}
 
-	dev->polling_mode = polling_req || client->irq <= 0;
-	if (!dev->polling_mode) {
-		pr_info("Requesting IRQ: %d\n", client->irq);
-		ret = request_irq(client->irq, most_irq_handler, 0,
-				  client->name, dev);
-		if (ret) {
-			pr_info("IRQ request failed: %d, falling back to polling\n",
-				ret);
-			dev->polling_mode = true;
-		}
-	}
-
-	if (dev->polling_mode)
-		pr_info("Using polling at rate: %d times/sec\n", scan_rate);
-
 	return 0;
 }
 
@@ -377,11 +383,7 @@ static int i2c_remove(struct i2c_client *client)
 {
 	struct hdm_i2c *dev = i2c_get_clientdata(client);
 
-	if (!dev->polling_mode)
-		free_irq(client->irq, dev);
-
 	most_deregister_interface(&dev->most_iface);
-	cancel_delayed_work_sync(&dev->rx.dwork);
 	kfree(dev);
 
 	return 0;
-- 
2.7.4

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

[PATCH 08/28] staging: most: i2c: do not wait in work function

[Christian Gromm]

This patch removes the function wait_event_interruptible from the
work function to avoid waiting.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/i2c/i2c.c | 54 ++++++++++++++++--------------------------
 1 file changed, 21 insertions(+), 33 deletions(-)

diff --git a/drivers/staging/most/i2c/i2c.c b/drivers/staging/most/i2c/i2c.c
index b382f09..a993e8e 100644
--- a/drivers/staging/most/i2c/i2c.c
+++ b/drivers/staging/most/i2c/i2c.c
@@ -11,7 +11,6 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/i2c.h>
-#include <linux/sched.h>
 #include <linux/interrupt.h>
 #include <linux/err.h>
 
@@ -47,9 +46,9 @@ struct hdm_i2c {
 	struct i2c_client *client;
 	struct rx {
 		struct delayed_work dwork;
-		wait_queue_head_t waitq;
 		struct list_head list;
 		struct mutex list_mutex;
+		bool int_disabled;
 	} rx;
 	char name[64];
 };
@@ -57,6 +56,7 @@ struct hdm_i2c {
 #define to_hdm(iface) container_of(iface, struct hdm_i2c, most_iface)
 
 static irqreturn_t most_irq_handler(int, void *);
+static void pending_rx_work(struct work_struct *);
 
 /**
  * configure_channel - called from MOST core to configure a channel
@@ -93,6 +93,7 @@ static int configure_channel(struct most_interface *most_iface,
 		dev->polling_mode = polling_req || dev->client->irq <= 0;
 		if (!dev->polling_mode) {
 			pr_info("Requesting IRQ: %d\n", dev->client->irq);
+			dev->rx.int_disabled = false;
 			ret = request_irq(dev->client->irq, most_irq_handler, 0,
 					  dev->client->name, dev);
 			if (ret) {
@@ -104,8 +105,6 @@ static int configure_channel(struct most_interface *most_iface,
 	}
 	if ((channel_config->direction == MOST_CH_RX) && (dev->polling_mode)) {
 		pr_info("Using polling at rate: %d times/sec\n", scan_rate);
-		schedule_delayed_work(&dev->rx.dwork,
-				      msecs_to_jiffies(MSEC_PER_SEC / 4));
 	}
 	dev->is_open[ch_idx] = true;
 
@@ -134,10 +133,16 @@ static int enqueue(struct most_interface *most_iface,
 
 	if (ch_idx == CH_RX) {
 		/* RX */
+		if (!dev->polling_mode)
+			disable_irq(dev->client->irq);
+		cancel_delayed_work_sync(&dev->rx.dwork);
 		mutex_lock(&dev->rx.list_mutex);
 		list_add_tail(&mbo->list, &dev->rx.list);
 		mutex_unlock(&dev->rx.list_mutex);
-		wake_up_interruptible(&dev->rx.waitq);
+		if (dev->rx.int_disabled || dev->polling_mode)
+			pending_rx_work(&dev->rx.dwork.work);
+		if (!dev->polling_mode)
+			enable_irq(dev->client->irq);
 	} else {
 		/* TX */
 		ret = i2c_master_send(dev->client, mbo->virt_address,
@@ -194,7 +199,6 @@ static int poison_channel(struct most_interface *most_iface,
 			mutex_lock(&dev->rx.list_mutex);
 		}
 		mutex_unlock(&dev->rx.list_mutex);
-		wake_up_interruptible(&dev->rx.waitq);
 	}
 
 	return 0;
@@ -204,7 +208,7 @@ static void do_rx_work(struct hdm_i2c *dev)
 {
 	struct mbo *mbo;
 	unsigned char msg[MAX_BUF_SIZE_CONTROL];
-	int ret, ch_idx = CH_RX;
+	int ret;
 	u16 pml, data_size;
 
 	/* Read PML (2 bytes) */
@@ -227,29 +231,7 @@ static void do_rx_work(struct hdm_i2c *dev)
 		return;
 	}
 
-	for (;;) {
-		/* Conditions to wait for: poisoned channel or free buffer
-		 * available for reading
-		 */
-		if (wait_event_interruptible(dev->rx.waitq,
-					     !dev->is_open[ch_idx] ||
-					     !list_empty(&dev->rx.list))) {
-			pr_err("wait_event_interruptible() failed\n");
-			return;
-		}
-
-		if (!dev->is_open[ch_idx])
-			return;
-
-		mutex_lock(&dev->rx.list_mutex);
-
-		/* list may be empty if poison or remove is called */
-		if (!list_empty(&dev->rx.list))
-			break;
-
-		mutex_unlock(&dev->rx.list_mutex);
-	}
-
+	mutex_lock(&dev->rx.list_mutex);
 	mbo = list_first_mbo(&dev->rx.list);
 	list_del(&mbo->list);
 	mutex_unlock(&dev->rx.list_mutex);
@@ -269,6 +251,13 @@ static void do_rx_work(struct hdm_i2c *dev)
 static void pending_rx_work(struct work_struct *work)
 {
 	struct hdm_i2c *dev = container_of(work, struct hdm_i2c, rx.dwork.work);
+	bool empty;
+
+	mutex_lock(&dev->rx.list_mutex);
+	empty = list_empty(&dev->rx.list);
+	mutex_unlock(&dev->rx.list_mutex);
+	if (empty)
+		return;
 
 	do_rx_work(dev);
 
@@ -278,6 +267,7 @@ static void pending_rx_work(struct work_struct *work)
 					      msecs_to_jiffies(MSEC_PER_SEC
 							       / scan_rate));
 	} else {
+		dev->rx.int_disabled = false;
 		enable_irq(dev->client->irq);
 	}
 }
@@ -305,7 +295,7 @@ static irqreturn_t most_irq_handler(int irq, void *_dev)
 	struct hdm_i2c *dev = _dev;
 
 	disable_irq_nosync(irq);
-
+	dev->rx.int_disabled = true;
 	schedule_delayed_work(&dev->rx.dwork, 0);
 
 	return IRQ_HANDLED;
@@ -354,7 +344,6 @@ static int i2c_probe(struct i2c_client *client, const struct i2c_device_id *id)
 
 	INIT_LIST_HEAD(&dev->rx.list);
 	mutex_init(&dev->rx.list_mutex);
-	init_waitqueue_head(&dev->rx.waitq);
 
 	INIT_DELAYED_WORK(&dev->rx.dwork, pending_rx_work);
 
@@ -407,7 +396,6 @@ static struct i2c_driver i2c_driver = {
 
 module_i2c_driver(i2c_driver);
 
-MODULE_AUTHOR("Jain Roy Ambi <JainRoy.Ambi@microchip.com>");
 MODULE_AUTHOR("Andrey Shvetsov <andrey.shvetsov@k2l.de>");
 MODULE_DESCRIPTION("I2C Hardware Dependent Module");
 MODULE_LICENSE("GPL");
-- 
2.7.4

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

[PATCH 09/28] staging: most: i2c: avoid polling in case of misconfig

[Christian Gromm]

This patch prevents the driver from falling back to polling mode
in case of IRQ misconfiguration.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/i2c/i2c.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/most/i2c/i2c.c b/drivers/staging/most/i2c/i2c.c
index a993e8e..e03cb6c 100644
--- a/drivers/staging/most/i2c/i2c.c
+++ b/drivers/staging/most/i2c/i2c.c
@@ -90,22 +90,24 @@ static int configure_channel(struct most_interface *most_iface,
 	}
 
 	if (channel_config->direction == MOST_CH_RX) {
-		dev->polling_mode = polling_req || dev->client->irq <= 0;
+		dev->polling_mode = polling_req;
 		if (!dev->polling_mode) {
-			pr_info("Requesting IRQ: %d\n", dev->client->irq);
+			if (dev->client->irq <= 0) {
+				pr_err("bad irq: %d\n", dev->client->irq);
+				return -ENOENT;
+			}
 			dev->rx.int_disabled = false;
 			ret = request_irq(dev->client->irq, most_irq_handler, 0,
 					  dev->client->name, dev);
 			if (ret) {
-				pr_info("IRQ request failed: %d, falling back to polling\n",
-					ret);
-				dev->polling_mode = true;
+				pr_err("request_irq(%d) failed: %d\n",
+				       dev->client->irq, ret);
+				return ret;
 			}
+		} else if (scan_rate) {
+			pr_info("polling rate is %d Hz\n", scan_rate);
 		}
 	}
-	if ((channel_config->direction == MOST_CH_RX) && (dev->polling_mode)) {
-		pr_info("Using polling at rate: %d times/sec\n", scan_rate);
-	}
 	dev->is_open[ch_idx] = true;
 
 	return 0;
-- 
2.7.4

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

Re: [PATCH 09/28] staging: most: i2c: avoid polling in case of misconfig

[Dan Carpenter]

On Tue, May 08, 2018 at 11:44:57AM +0200, Christian Gromm wrote:
> This patch prevents the driver from falling back to polling mode
> in case of IRQ misconfiguration.
> 
> Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> ---
>  drivers/staging/most/i2c/i2c.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/most/i2c/i2c.c b/drivers/staging/most/i2c/i2c.c
> index a993e8e..e03cb6c 100644
> --- a/drivers/staging/most/i2c/i2c.c
> +++ b/drivers/staging/most/i2c/i2c.c
> @@ -90,22 +90,24 @@ static int configure_channel(struct most_interface *most_iface,
>  	}
>  
>  	if (channel_config->direction == MOST_CH_RX) {
> -		dev->polling_mode = polling_req || dev->client->irq <= 0;
> +		dev->polling_mode = polling_req;
>  		if (!dev->polling_mode) {
> -			pr_info("Requesting IRQ: %d\n", dev->client->irq);
> +			if (dev->client->irq <= 0) {
> +				pr_err("bad irq: %d\n", dev->client->irq);
> +				return -ENOENT;
> +			}

I was wondering about this at the time...  It would have been better to
fold this in with the earlier patch and it's pretty easy to fold patches
together with `git rebase -i`.

regards,
dan carpenter


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

Re: [PATCH 09/28] staging: most: i2c: avoid polling in case of misconfig

[Christian Gromm]

On 09.05.2018 15:46, Dan Carpenter wrote:
> On Tue, May 08, 2018 at 11:44:57AM +0200, Christian Gromm wrote:
>> This patch prevents the driver from falling back to polling mode
>> in case of IRQ misconfiguration.
>>
>> Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
>> ---
>>   drivers/staging/most/i2c/i2c.c | 18 ++++++++++--------
>>   1 file changed, 10 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/staging/most/i2c/i2c.c b/drivers/staging/most/i2c/i2c.c
>> index a993e8e..e03cb6c 100644
>> --- a/drivers/staging/most/i2c/i2c.c
>> +++ b/drivers/staging/most/i2c/i2c.c
>> @@ -90,22 +90,24 @@ static int configure_channel(struct most_interface *most_iface,
>>   	}
>>   
>>   	if (channel_config->direction == MOST_CH_RX) {
>> -		dev->polling_mode = polling_req || dev->client->irq <= 0;
>> +		dev->polling_mode = polling_req;
>>   		if (!dev->polling_mode) {
>> -			pr_info("Requesting IRQ: %d\n", dev->client->irq);
>> +			if (dev->client->irq <= 0) {
>> +				pr_err("bad irq: %d\n", dev->client->irq);
>> +				return -ENOENT;
>> +			}
> 
> I was wondering about this at the time...  It would have been better to
> fold this in with the earlier patch and it's pretty easy to fold patches
> together with `git rebase -i`.

Yes probably, but isn't doing two things at a time a bad thing to do?

thanks,
Chris

> 
> regards,
> dan carpenter
> 
> 

[PATCH 10/28] staging: most: i2c: prevent zero delay polling

[Christian Gromm]

This patch avoids that a configured scan_rate of more than MSEC_PER_SEC
might result in a polling delay of zero.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/i2c/i2c.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/most/i2c/i2c.c b/drivers/staging/most/i2c/i2c.c
index e03cb6c..30d18cc 100644
--- a/drivers/staging/most/i2c/i2c.c
+++ b/drivers/staging/most/i2c/i2c.c
@@ -49,6 +49,7 @@ struct hdm_i2c {
 		struct list_head list;
 		struct mutex list_mutex;
 		bool int_disabled;
+		unsigned int delay;
 	} rx;
 	char name[64];
 };
@@ -75,6 +76,7 @@ static int configure_channel(struct most_interface *most_iface,
 {
 	int ret;
 	struct hdm_i2c *dev = to_hdm(most_iface);
+	unsigned int delay;
 
 	BUG_ON(ch_idx < 0 || ch_idx >= NUM_CHANNELS);
 	BUG_ON(dev->is_open[ch_idx]);
@@ -105,6 +107,8 @@ static int configure_channel(struct most_interface *most_iface,
 				return ret;
 			}
 		} else if (scan_rate) {
+			delay = msecs_to_jiffies(MSEC_PER_SEC / scan_rate);
+			dev->rx.delay = delay ? delay : 1;
 			pr_info("polling rate is %d Hz\n", scan_rate);
 		}
 	}
@@ -265,9 +269,7 @@ static void pending_rx_work(struct work_struct *work)
 
 	if (dev->polling_mode) {
 		if (dev->is_open[CH_RX] && scan_rate)
-			schedule_delayed_work(&dev->rx.dwork,
-					      msecs_to_jiffies(MSEC_PER_SEC
-							       / scan_rate));
+			schedule_delayed_work(&dev->rx.dwork, dev->rx.delay);
 	} else {
 		dev->rx.int_disabled = false;
 		enable_irq(dev->client->irq);
-- 
2.7.4

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

[PATCH 11/28] staging: most: i2c: trace real polling rate

[Christian Gromm]

The real polling rate depends on the CONFIG_HZ and may differ from the
required polling rate.

Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/i2c/i2c.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/most/i2c/i2c.c b/drivers/staging/most/i2c/i2c.c
index 30d18cc..39b1590 100644
--- a/drivers/staging/most/i2c/i2c.c
+++ b/drivers/staging/most/i2c/i2c.c
@@ -76,7 +76,7 @@ static int configure_channel(struct most_interface *most_iface,
 {
 	int ret;
 	struct hdm_i2c *dev = to_hdm(most_iface);
-	unsigned int delay;
+	unsigned int delay, pr;
 
 	BUG_ON(ch_idx < 0 || ch_idx >= NUM_CHANNELS);
 	BUG_ON(dev->is_open[ch_idx]);
@@ -109,7 +109,8 @@ static int configure_channel(struct most_interface *most_iface,
 		} else if (scan_rate) {
 			delay = msecs_to_jiffies(MSEC_PER_SEC / scan_rate);
 			dev->rx.delay = delay ? delay : 1;
-			pr_info("polling rate is %d Hz\n", scan_rate);
+			pr = MSEC_PER_SEC / jiffies_to_msecs(dev->rx.delay);
+			pr_info("polling rate is %u Hz\n", pr);
 		}
 	}
 	dev->is_open[ch_idx] = true;
-- 
2.7.4

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

[PATCH 12/28] staging: most: i2c: remove redundant is_open

[Christian Gromm]

The variable is_open is checked only in the work function
pending_rx_work() that is only active between the calls
configure_channel() and poison_channel().

Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/i2c/i2c.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/staging/most/i2c/i2c.c b/drivers/staging/most/i2c/i2c.c
index 39b1590..8edced9 100644
--- a/drivers/staging/most/i2c/i2c.c
+++ b/drivers/staging/most/i2c/i2c.c
@@ -39,7 +39,6 @@ module_param(scan_rate, int, 0644);
 MODULE_PARM_DESC(scan_rate, "Polling rate in times/sec. Default = 100");
 
 struct hdm_i2c {
-	bool is_open[NUM_CHANNELS];
 	bool polling_mode;
 	struct most_interface most_iface;
 	struct most_channel_capability capabilities[NUM_CHANNELS];
@@ -79,7 +78,6 @@ static int configure_channel(struct most_interface *most_iface,
 	unsigned int delay, pr;
 
 	BUG_ON(ch_idx < 0 || ch_idx >= NUM_CHANNELS);
-	BUG_ON(dev->is_open[ch_idx]);
 
 	if (channel_config->data_type != MOST_CH_CONTROL) {
 		pr_err("bad data type for channel %d\n", ch_idx);
@@ -113,7 +111,6 @@ static int configure_channel(struct most_interface *most_iface,
 			pr_info("polling rate is %u Hz\n", pr);
 		}
 	}
-	dev->is_open[ch_idx] = true;
 
 	return 0;
 }
@@ -136,7 +133,6 @@ static int enqueue(struct most_interface *most_iface,
 	int ret;
 
 	BUG_ON(ch_idx < 0 || ch_idx >= NUM_CHANNELS);
-	BUG_ON(!dev->is_open[ch_idx]);
 
 	if (ch_idx == CH_RX) {
 		/* RX */
@@ -184,9 +180,6 @@ static int poison_channel(struct most_interface *most_iface,
 	struct mbo *mbo;
 
 	BUG_ON(ch_idx < 0 || ch_idx >= NUM_CHANNELS);
-	BUG_ON(!dev->is_open[ch_idx]);
-
-	dev->is_open[ch_idx] = false;
 
 	if (ch_idx == CH_RX) {
 		if (!dev->polling_mode)
@@ -269,7 +262,7 @@ static void pending_rx_work(struct work_struct *work)
 	do_rx_work(dev);
 
 	if (dev->polling_mode) {
-		if (dev->is_open[CH_RX] && scan_rate)
+		if (scan_rate)
 			schedule_delayed_work(&dev->rx.dwork, dev->rx.delay);
 	} else {
 		dev->rx.int_disabled = false;
@@ -329,7 +322,6 @@ static int i2c_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		 client->adapter->nr, client->addr);
 
 	for (i = 0; i < NUM_CHANNELS; i++) {
-		dev->is_open[i] = false;
 		dev->capabilities[i].data_type = MOST_CH_CONTROL;
 		dev->capabilities[i].num_buffers_packet = MAX_BUFFERS_CONTROL;
 		dev->capabilities[i].buffer_size_packet = MAX_BUF_SIZE_CONTROL;
-- 
2.7.4

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

[PATCH 13/28] staging: most: i2c: remove redundant list_mutex

[Christian Gromm]

The elements of the dev->rx.list are consumed in the pending_rx_work and
populated in the function enqueue() that cancels the pending_rx_work.

The function enqueue() and poison_channel() do not race anyway.

Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/i2c/i2c.c | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/drivers/staging/most/i2c/i2c.c b/drivers/staging/most/i2c/i2c.c
index 8edced9..f0d5b22 100644
--- a/drivers/staging/most/i2c/i2c.c
+++ b/drivers/staging/most/i2c/i2c.c
@@ -46,7 +46,6 @@ struct hdm_i2c {
 	struct rx {
 		struct delayed_work dwork;
 		struct list_head list;
-		struct mutex list_mutex;
 		bool int_disabled;
 		unsigned int delay;
 	} rx;
@@ -139,9 +138,7 @@ static int enqueue(struct most_interface *most_iface,
 		if (!dev->polling_mode)
 			disable_irq(dev->client->irq);
 		cancel_delayed_work_sync(&dev->rx.dwork);
-		mutex_lock(&dev->rx.list_mutex);
 		list_add_tail(&mbo->list, &dev->rx.list);
-		mutex_unlock(&dev->rx.list_mutex);
 		if (dev->rx.int_disabled || dev->polling_mode)
 			pending_rx_work(&dev->rx.dwork.work);
 		if (!dev->polling_mode)
@@ -186,19 +183,14 @@ static int poison_channel(struct most_interface *most_iface,
 			free_irq(dev->client->irq, dev);
 		cancel_delayed_work_sync(&dev->rx.dwork);
 
-		mutex_lock(&dev->rx.list_mutex);
 		while (!list_empty(&dev->rx.list)) {
 			mbo = list_first_mbo(&dev->rx.list);
 			list_del(&mbo->list);
-			mutex_unlock(&dev->rx.list_mutex);
 
 			mbo->processed_length = 0;
 			mbo->status = MBO_E_CLOSE;
 			mbo->complete(mbo);
-
-			mutex_lock(&dev->rx.list_mutex);
 		}
-		mutex_unlock(&dev->rx.list_mutex);
 	}
 
 	return 0;
@@ -231,10 +223,8 @@ static void do_rx_work(struct hdm_i2c *dev)
 		return;
 	}
 
-	mutex_lock(&dev->rx.list_mutex);
 	mbo = list_first_mbo(&dev->rx.list);
 	list_del(&mbo->list);
-	mutex_unlock(&dev->rx.list_mutex);
 
 	mbo->processed_length = min(data_size, mbo->buffer_length);
 	memcpy(mbo->virt_address, msg, mbo->processed_length);
@@ -251,12 +241,8 @@ static void do_rx_work(struct hdm_i2c *dev)
 static void pending_rx_work(struct work_struct *work)
 {
 	struct hdm_i2c *dev = container_of(work, struct hdm_i2c, rx.dwork.work);
-	bool empty;
 
-	mutex_lock(&dev->rx.list_mutex);
-	empty = list_empty(&dev->rx.list);
-	mutex_unlock(&dev->rx.list_mutex);
-	if (empty)
+	if (list_empty(&dev->rx.list))
 		return;
 
 	do_rx_work(dev);
@@ -340,7 +326,6 @@ static int i2c_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	dev->most_iface.poison_channel = poison_channel;
 
 	INIT_LIST_HEAD(&dev->rx.list);
-	mutex_init(&dev->rx.list_mutex);
 
 	INIT_DELAYED_WORK(&dev->rx.dwork, pending_rx_work);
 
-- 
2.7.4

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

[PATCH 14/28] staging: most: i2c: reduce parameters inconsistency

[Christian Gromm]

Currently, there are two module parameters for the i2c driver:
  - polling_req: boolean irq/polling mode;
  - scan_rate: polling rate, that is used in the case where the polling
    mode is active

This model is misconfiguration-prone.  For example, it is possible to
select polling mode with the zero polling rate or configure non-zero
polling rate in a combination with the IRQ mode.

This patch replaces the 'polling_req' and 'scan_rate' by the
'polling_rate', where the value zero means the interrupt driven mode and
other values are used as the polling rate in the polling mode.

Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/i2c/i2c.c | 33 ++++++++++++---------------------
 1 file changed, 12 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/most/i2c/i2c.c b/drivers/staging/most/i2c/i2c.c
index f0d5b22..4a4fc10 100644
--- a/drivers/staging/most/i2c/i2c.c
+++ b/drivers/staging/most/i2c/i2c.c
@@ -28,18 +28,11 @@ enum { CH_RX, CH_TX, NUM_CHANNELS };
 #define list_first_mbo(ptr) \
 	list_first_entry(ptr, struct mbo, list)
 
-/* IRQ / Polling option */
-static bool polling_req;
-module_param(polling_req, bool, 0444);
-MODULE_PARM_DESC(polling_req, "Request Polling. Default = 0 (use irq)");
-
-/* Polling Rate */
-static int scan_rate = 100;
-module_param(scan_rate, int, 0644);
-MODULE_PARM_DESC(scan_rate, "Polling rate in times/sec. Default = 100");
+static unsigned int polling_rate;
+module_param(polling_rate, uint, 0644);
+MODULE_PARM_DESC(polling_rate, "Polling rate [Hz]. Default = 0 (use IRQ)");
 
 struct hdm_i2c {
-	bool polling_mode;
 	struct most_interface most_iface;
 	struct most_channel_capability capabilities[NUM_CHANNELS];
 	struct i2c_client *client;
@@ -89,8 +82,7 @@ static int configure_channel(struct most_interface *most_iface,
 	}
 
 	if (channel_config->direction == MOST_CH_RX) {
-		dev->polling_mode = polling_req;
-		if (!dev->polling_mode) {
+		if (!polling_rate) {
 			if (dev->client->irq <= 0) {
 				pr_err("bad irq: %d\n", dev->client->irq);
 				return -ENOENT;
@@ -103,8 +95,8 @@ static int configure_channel(struct most_interface *most_iface,
 				       dev->client->irq, ret);
 				return ret;
 			}
-		} else if (scan_rate) {
-			delay = msecs_to_jiffies(MSEC_PER_SEC / scan_rate);
+		} else {
+			delay = msecs_to_jiffies(MSEC_PER_SEC / polling_rate);
 			dev->rx.delay = delay ? delay : 1;
 			pr = MSEC_PER_SEC / jiffies_to_msecs(dev->rx.delay);
 			pr_info("polling rate is %u Hz\n", pr);
@@ -135,13 +127,13 @@ static int enqueue(struct most_interface *most_iface,
 
 	if (ch_idx == CH_RX) {
 		/* RX */
-		if (!dev->polling_mode)
+		if (!polling_rate)
 			disable_irq(dev->client->irq);
 		cancel_delayed_work_sync(&dev->rx.dwork);
 		list_add_tail(&mbo->list, &dev->rx.list);
-		if (dev->rx.int_disabled || dev->polling_mode)
+		if (dev->rx.int_disabled || polling_rate)
 			pending_rx_work(&dev->rx.dwork.work);
-		if (!dev->polling_mode)
+		if (!polling_rate)
 			enable_irq(dev->client->irq);
 	} else {
 		/* TX */
@@ -179,7 +171,7 @@ static int poison_channel(struct most_interface *most_iface,
 	BUG_ON(ch_idx < 0 || ch_idx >= NUM_CHANNELS);
 
 	if (ch_idx == CH_RX) {
-		if (!dev->polling_mode)
+		if (!polling_rate)
 			free_irq(dev->client->irq, dev);
 		cancel_delayed_work_sync(&dev->rx.dwork);
 
@@ -247,9 +239,8 @@ static void pending_rx_work(struct work_struct *work)
 
 	do_rx_work(dev);
 
-	if (dev->polling_mode) {
-		if (scan_rate)
-			schedule_delayed_work(&dev->rx.dwork, dev->rx.delay);
+	if (polling_rate) {
+		schedule_delayed_work(&dev->rx.dwork, dev->rx.delay);
 	} else {
 		dev->rx.int_disabled = false;
 		enable_irq(dev->client->irq);
-- 
2.7.4

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

[PATCH 15/28] staging: most: make interface drivers allocate coherent memory

[Christian Gromm]

On arm64/aarch64 architectures the allocation of coherent memory needs a
device that has the dma_ops properly set. That's why the core module of
the MOST driver is no longer able to allocate this type or memory. This
patch moves the allocation process down to the interface drivers where
the proper devices exist (e.g. platform device or USB system software).

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/core.c      | 17 +++++++++++------
 drivers/staging/most/core.h      |  2 ++
 drivers/staging/most/dim2/dim2.c | 12 ++++++++++++
 drivers/staging/most/usb/usb.c   | 18 ++++++++++++++++++
 4 files changed, 43 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/most/core.c b/drivers/staging/most/core.c
index db3c7e2..f4c4646 100644
--- a/drivers/staging/most/core.c
+++ b/drivers/staging/most/core.c
@@ -111,8 +111,10 @@ static void most_free_mbo_coherent(struct mbo *mbo)
 	struct most_channel *c = mbo->context;
 	u16 const coherent_buf_size = c->cfg.buffer_size + c->cfg.extra_len;
 
-	dma_free_coherent(NULL, coherent_buf_size, mbo->virt_address,
-			  mbo->bus_address);
+	if (c->iface->dma_free)
+		c->iface->dma_free(mbo, coherent_buf_size);
+	else
+		kfree(mbo->virt_address);
 	kfree(mbo);
 	if (atomic_sub_and_test(1, &c->mbo_ref))
 		complete(&c->cleanup);
@@ -988,10 +990,13 @@ static int arm_mbo_chain(struct most_channel *c, int dir,
 		mbo->context = c;
 		mbo->ifp = c->iface;
 		mbo->hdm_channel_id = c->channel_id;
-		mbo->virt_address = dma_alloc_coherent(NULL,
-						       coherent_buf_size,
-						       &mbo->bus_address,
-						       GFP_KERNEL);
+		if (c->iface->dma_alloc) {
+			mbo->virt_address =
+				c->iface->dma_alloc(mbo, coherent_buf_size);
+		} else {
+			mbo->virt_address =
+				kzalloc(coherent_buf_size, GFP_KERNEL);
+		}
 		if (!mbo->virt_address)
 			goto release_mbo;
 
diff --git a/drivers/staging/most/core.h b/drivers/staging/most/core.h
index 5b184e6..7a3c70b 100644
--- a/drivers/staging/most/core.h
+++ b/drivers/staging/most/core.h
@@ -235,6 +235,8 @@ struct most_interface {
 	const char *description;
 	unsigned int num_channels;
 	struct most_channel_capability *channel_vector;
+	void *(*dma_alloc)(struct mbo *mbo, u32 size);
+	void (*dma_free)(struct mbo *mbo, u32 size);
 	int (*configure)(struct most_interface *iface, int channel_idx,
 			 struct most_channel_config *channel_config);
 	int (*enqueue)(struct most_interface *iface, int channel_idx,
diff --git a/drivers/staging/most/dim2/dim2.c b/drivers/staging/most/dim2/dim2.c
index d15867a..3c385b3 100644
--- a/drivers/staging/most/dim2/dim2.c
+++ b/drivers/staging/most/dim2/dim2.c
@@ -703,6 +703,16 @@ static int poison_channel(struct most_interface *most_iface, int ch_idx)
 	return ret;
 }
 
+static void *dma_alloc(struct mbo *mbo, u32 size)
+{
+	return dma_alloc_coherent(NULL, size, &mbo->bus_address, GFP_KERNEL);
+}
+
+static void dma_free(struct mbo *mbo, u32 size)
+{
+	dma_free_coherent(NULL, size, mbo->virt_address, mbo->bus_address);
+}
+
 /*
  * dim2_probe - dim2 probe handler
  * @pdev: platform device structure
@@ -800,6 +810,8 @@ static int dim2_probe(struct platform_device *pdev)
 	dev->most_iface.channel_vector = dev->capabilities;
 	dev->most_iface.configure = configure_channel;
 	dev->most_iface.enqueue = enqueue;
+	dev->most_iface.dma_alloc = dma_alloc;
+	dev->most_iface.dma_free = dma_free;
 	dev->most_iface.poison_channel = poison_channel;
 	dev->most_iface.request_netinfo = request_netinfo;
 	dev->dev.init_name = "dim2_state";
diff --git a/drivers/staging/most/usb/usb.c b/drivers/staging/most/usb/usb.c
index 31f184c..5ed1dcc 100644
--- a/drivers/staging/most/usb/usb.c
+++ b/drivers/staging/most/usb/usb.c
@@ -618,6 +618,22 @@ static int hdm_enqueue(struct most_interface *iface, int channel,
 	return retval;
 }
 
+static void *hdm_dma_alloc(struct mbo *mbo, u32 size)
+{
+	struct most_dev *mdev = to_mdev(mbo->ifp);
+
+	return usb_alloc_coherent(mdev->usb_device, size, GFP_KERNEL,
+				  &mbo->bus_address);
+}
+
+static void hdm_dma_free(struct mbo *mbo, u32 size)
+{
+	struct most_dev *mdev = to_mdev(mbo->ifp);
+
+	usb_free_coherent(mdev->usb_device, size, mbo->virt_address,
+			  mbo->bus_address);
+}
+
 /**
  * hdm_configure_channel - receive channel configuration from core
  * @iface: interface
@@ -1032,6 +1048,8 @@ hdm_probe(struct usb_interface *interface, const struct usb_device_id *id)
 	mdev->iface.request_netinfo = hdm_request_netinfo;
 	mdev->iface.enqueue = hdm_enqueue;
 	mdev->iface.poison_channel = hdm_poison_channel;
+	mdev->iface.dma_alloc = hdm_dma_alloc;
+	mdev->iface.dma_free = hdm_dma_free;
 	mdev->iface.description = mdev->description;
 	mdev->iface.num_channels = num_endpoints;
 
-- 
2.7.4

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

[PATCH 16/28] staging: most: sound: call snd_card_new with struct device

[Christian Gromm]

This patch is needed as function snd_card_new needs a valid
parent device. Passing a NULL pointer leads to kernel Ooops.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/core.h        | 1 +
 drivers/staging/most/sound/sound.c | 2 +-
 drivers/staging/most/usb/usb.c     | 1 +
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/most/core.h b/drivers/staging/most/core.h
index 7a3c70b..64cc02f 100644
--- a/drivers/staging/most/core.h
+++ b/drivers/staging/most/core.h
@@ -230,6 +230,7 @@ struct mbo {
  */
 struct most_interface {
 	struct device dev;
+	struct device *driver_dev;
 	struct module *mod;
 	enum most_interface_type interface;
 	const char *description;
diff --git a/drivers/staging/most/sound/sound.c b/drivers/staging/most/sound/sound.c
index 18f7224..04c1832 100644
--- a/drivers/staging/most/sound/sound.c
+++ b/drivers/staging/most/sound/sound.c
@@ -590,7 +590,7 @@ static int audio_probe_channel(struct most_interface *iface, int channel_id,
 	if (ret < 0)
 		return ret;
 
-	ret = snd_card_new(NULL, -1, card_name, THIS_MODULE,
+	ret = snd_card_new(&iface->dev, -1, card_name, THIS_MODULE,
 			   sizeof(*channel), &card);
 	if (ret < 0)
 		return ret;
diff --git a/drivers/staging/most/usb/usb.c b/drivers/staging/most/usb/usb.c
index 5ed1dcc..f187260 100644
--- a/drivers/staging/most/usb/usb.c
+++ b/drivers/staging/most/usb/usb.c
@@ -1043,6 +1043,7 @@ hdm_probe(struct usb_interface *interface, const struct usb_device_id *id)
 	mdev->link_stat_timer.expires = jiffies + (2 * HZ);
 
 	mdev->iface.mod = hdm_usb_fops.owner;
+	mdev->iface.driver_dev = &interface->dev;
 	mdev->iface.interface = ITYPE_USB;
 	mdev->iface.configure = hdm_configure_channel;
 	mdev->iface.request_netinfo = hdm_request_netinfo;
-- 
2.7.4

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

[PATCH 17/28] staging: most: cdev: avoid warning about potentially uninitialized variable

[Christian Gromm]

This patch avoids the warning that the pointer mbo might be used
uninitialized that some environmens throw.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/cdev/cdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/most/cdev/cdev.c b/drivers/staging/most/cdev/cdev.c
index 4d7fce8..89d7fc7 100644
--- a/drivers/staging/most/cdev/cdev.c
+++ b/drivers/staging/most/cdev/cdev.c
@@ -242,7 +242,7 @@ static ssize_t
 comp_read(struct file *filp, char __user *buf, size_t count, loff_t *offset)
 {
 	size_t to_copy, not_copied, copied;
-	struct mbo *mbo;
+	struct mbo *mbo = NULL;
 	struct comp_channel *c = filp->private_data;
 
 	mutex_lock(&c->io_mutex);
-- 
2.7.4

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

[PATCH 18/28] staging: most: cdev: fix chrdev_region leak

[Christian Gromm]

The function unregister_chrdev_region is called with a different counter
as the alloc_chrdev_region. To fix this, this patch introduces the
constant CHRDEV_REGION_SIZE that is used in both functions.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/cdev/cdev.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/most/cdev/cdev.c b/drivers/staging/most/cdev/cdev.c
index 89d7fc7..9ce7fd2 100644
--- a/drivers/staging/most/cdev/cdev.c
+++ b/drivers/staging/most/cdev/cdev.c
@@ -18,6 +18,8 @@
 #include <linux/idr.h>
 #include "most/core.h"
 
+#define CHRDEV_REGION_SIZE 50
+
 static struct cdev_component {
 	dev_t devno;
 	struct ida minor_id;
@@ -513,7 +515,7 @@ static int __init mod_init(void)
 	spin_lock_init(&ch_list_lock);
 	ida_init(&comp.minor_id);
 
-	err = alloc_chrdev_region(&comp.devno, 0, 50, "cdev");
+	err = alloc_chrdev_region(&comp.devno, 0, CHRDEV_REGION_SIZE, "cdev");
 	if (err < 0)
 		goto dest_ida;
 	comp.major = MAJOR(comp.devno);
@@ -523,7 +525,7 @@ static int __init mod_init(void)
 	return 0;
 
 free_cdev:
-	unregister_chrdev_region(comp.devno, 1);
+	unregister_chrdev_region(comp.devno, CHRDEV_REGION_SIZE);
 dest_ida:
 	ida_destroy(&comp.minor_id);
 	class_destroy(comp.class);
-- 
2.7.4

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

[PATCH 19/28] staging: most: usb: add ep number to log

[Christian Gromm]

This patch adds the endpoint number of the USB pipe that reports to be
broken into the log message. It is needed to make debugging for
applications more comfortable.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/usb/usb.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/most/usb/usb.c b/drivers/staging/most/usb/usb.c
index f187260..3126b69 100644
--- a/drivers/staging/most/usb/usb.c
+++ b/drivers/staging/most/usb/usb.c
@@ -354,7 +354,8 @@ static void hdm_write_completion(struct urb *urb)
 			mbo->status = MBO_SUCCESS;
 			break;
 		case -EPIPE:
-			dev_warn(dev, "Broken OUT pipe detected\n");
+			dev_warn(dev, "Broken pipe on ep%02x\n",
+				 mdev->ep_address[channel]);
 			mdev->is_channel_healthy[channel] = false;
 			mdev->clear_work[channel].pipe = urb->pipe;
 			schedule_work(&mdev->clear_work[channel].ws);
@@ -507,7 +508,8 @@ static void hdm_read_completion(struct urb *urb)
 			}
 			break;
 		case -EPIPE:
-			dev_warn(dev, "Broken IN pipe detected\n");
+			dev_warn(dev, "Broken pipe on ep%02x\n",
+				 mdev->ep_address[channel]);
 			mdev->is_channel_healthy[channel] = false;
 			mdev->clear_work[channel].pipe = urb->pipe;
 			schedule_work(&mdev->clear_work[channel].ws);
@@ -517,7 +519,8 @@ static void hdm_read_completion(struct urb *urb)
 			mbo->status = MBO_E_CLOSE;
 			break;
 		case -EOVERFLOW:
-			dev_warn(dev, "Babble on IN pipe detected\n");
+			dev_warn(dev, "Babble on ep%02x\n",
+				 mdev->ep_address[channel]);
 			break;
 		}
 	}
-- 
2.7.4

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

[PATCH 20/28] staging: most: cdev: fix function return value

[Christian Gromm]

The function ch_get_mbo declares its return value as type bool,
but returns a pointer to mbo.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/cdev/cdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/most/cdev/cdev.c b/drivers/staging/most/cdev/cdev.c
index 9ce7fd2..8e76525 100644
--- a/drivers/staging/most/cdev/cdev.c
+++ b/drivers/staging/most/cdev/cdev.c
@@ -53,7 +53,7 @@ static inline bool ch_has_mbo(struct comp_channel *c)
 	return channel_has_mbo(c->iface, c->channel_id, &comp.cc) > 0;
 }
 
-static inline bool ch_get_mbo(struct comp_channel *c, struct mbo **mbo)
+static inline struct mbo *ch_get_mbo(struct comp_channel *c, struct mbo **mbo)
 {
 	if (!kfifo_peek(&c->fifo, mbo)) {
 		*mbo = most_get_mbo(c->iface, c->channel_id, &comp.cc);
-- 
2.7.4

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

[PATCH 21/28] staging: most: dim2: fix startup sequence

[Christian Gromm]

Platform specific initialization (data->init) has to be done before
calling dim_startup to start the DIM2 IP.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/dim2/dim2.c | 90 +++++++++++++++++-----------------------
 1 file changed, 39 insertions(+), 51 deletions(-)

diff --git a/drivers/staging/most/dim2/dim2.c b/drivers/staging/most/dim2/dim2.c
index 3c385b3..e60efa0 100644
--- a/drivers/staging/most/dim2/dim2.c
+++ b/drivers/staging/most/dim2/dim2.c
@@ -150,38 +150,6 @@ void dimcb_on_error(u8 error_id, const char *error_message)
 }
 
 /**
- * startup_dim - initialize the dim2 interface
- * @pdev: platform device
- */
-static int startup_dim(struct platform_device *pdev)
-{
-	struct dim2_hdm *dev = platform_get_drvdata(pdev);
-	struct dim2_platform_data *pdata = pdev->dev.platform_data;
-	u8 hal_ret;
-	int ret;
-
-	if (!pdata) {
-		pr_err("missing platform data\n");
-		return -EINVAL;
-	}
-
-	ret = pdata->init ? pdata->init(pdata, dev->io_base) : 0;
-	if (ret)
-		return ret;
-
-	pr_info("sync: num of frames per sub-buffer: %u\n", fcnt);
-	hal_ret = dim_startup(dev->io_base, pdata->clk_speed, fcnt);
-	if (hal_ret != DIM_NO_ERROR) {
-		pr_err("dim_startup failed: %d\n", hal_ret);
-		if (pdata && pdata->destroy)
-			pdata->destroy(pdata);
-		return -ENODEV;
-	}
-
-	return 0;
-}
-
-/**
  * try_start_dim_transfer - try to transfer a buffer on a channel
  * @hdm_ch: channel specific data
  *
@@ -722,9 +690,11 @@ static void dma_free(struct mbo *mbo, u32 size)
  */
 static int dim2_probe(struct platform_device *pdev)
 {
+	struct dim2_platform_data *pdata = pdev->dev.platform_data;
 	struct dim2_hdm *dev;
 	struct resource *res;
 	int ret, i;
+	u8 hal_ret;
 	int irq;
 
 	dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
@@ -739,38 +709,59 @@ static int dim2_probe(struct platform_device *pdev)
 	if (IS_ERR(dev->io_base))
 		return PTR_ERR(dev->io_base);
 
+	if (!pdata) {
+		dev_err(&pdev->dev, "missing platform data\n");
+		return -EINVAL;
+	}
+
+	ret = pdata->init ? pdata->init(pdata, dev->io_base) : 0;
+	if (ret)
+		return ret;
+
+	dev_info(&pdev->dev, "sync: num of frames per sub-buffer: %u\n", fcnt);
+	hal_ret = dim_startup(dev->io_base, pdata->clk_speed, fcnt);
+	if (hal_ret != DIM_NO_ERROR) {
+		dev_err(&pdev->dev, "dim_startup failed: %d\n", hal_ret);
+		ret = -ENODEV;
+		goto err_bsp_destroy;
+	}
+
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0) {
 		dev_err(&pdev->dev, "failed to get ahb0_int irq: %d\n", irq);
-		return irq;
+		ret = irq;
+		goto err_shutdown_dim;
 	}
 
 	ret = devm_request_irq(&pdev->dev, irq, dim2_ahb_isr, 0,
 			       "dim2_ahb0_int", dev);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to request ahb0_int irq %d\n", irq);
-		return ret;
+		goto err_shutdown_dim;
 	}
 
 	irq = platform_get_irq(pdev, 1);
 	if (irq < 0) {
 		dev_err(&pdev->dev, "failed to get mlb_int irq: %d\n", irq);
-		return irq;
+		ret = irq;
+		goto err_shutdown_dim;
 	}
 
 	ret = devm_request_irq(&pdev->dev, irq, dim2_mlb_isr, 0,
 			       "dim2_mlb_int", dev);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to request mlb_int irq %d\n", irq);
-		return ret;
+		goto err_shutdown_dim;
 	}
 
 	init_waitqueue_head(&dev->netinfo_waitq);
 	dev->deliver_netinfo = 0;
-	dev->netinfo_task = kthread_run(&deliver_netinfo_thread, (void *)dev,
+	dev->netinfo_task = kthread_run(&deliver_netinfo_thread, dev,
 					"dim2_netinfo");
-	if (IS_ERR(dev->netinfo_task))
-		return PTR_ERR(dev->netinfo_task);
+	if (IS_ERR(dev->netinfo_task)) {
+		ret = PTR_ERR(dev->netinfo_task);
+		goto err_shutdown_dim;
+	}
 
 	for (i = 0; i < DMA_CHANNELS; i++) {
 		struct most_channel_capability *cap = dev->capabilities + i;
@@ -829,20 +820,17 @@ static int dim2_probe(struct platform_device *pdev)
 		goto err_unreg_iface;
 	}
 
-	ret = startup_dim(pdev);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to initialize DIM2\n");
-		goto err_destroy_bus;
-	}
-
 	return 0;
 
-err_destroy_bus:
-	dim2_sysfs_destroy(&dev->dev);
 err_unreg_iface:
 	most_deregister_interface(&dev->most_iface);
 err_stop_thread:
 	kthread_stop(dev->netinfo_task);
+err_shutdown_dim:
+	dim_shutdown();
+err_bsp_destroy:
+	if (pdata && pdata->destroy)
+		pdata->destroy(pdata);
 
 	return ret;
 }
@@ -859,6 +847,10 @@ static int dim2_remove(struct platform_device *pdev)
 	struct dim2_platform_data *pdata = pdev->dev.platform_data;
 	unsigned long flags;
 
+	dim2_sysfs_destroy(&dev->dev);
+	most_deregister_interface(&dev->most_iface);
+	kthread_stop(dev->netinfo_task);
+
 	spin_lock_irqsave(&dim_lock, flags);
 	dim_shutdown();
 	spin_unlock_irqrestore(&dim_lock, flags);
@@ -866,10 +858,6 @@ static int dim2_remove(struct platform_device *pdev)
 	if (pdata && pdata->destroy)
 		pdata->destroy(pdata);
 
-	dim2_sysfs_destroy(&dev->dev);
-	most_deregister_interface(&dev->most_iface);
-	kthread_stop(dev->netinfo_task);
-
 	/*
 	 * break link to local platform_device_id struct
 	 * to prevent crash by unload platform device module
-- 
2.7.4

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

[PATCH 22/28] staging: most: cdev: fix race condition

[Christian Gromm]

This patch fixes a race condition between the functions disconnect and poll.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/cdev/cdev.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/most/cdev/cdev.c b/drivers/staging/most/cdev/cdev.c
index 8e76525..4569838 100644
--- a/drivers/staging/most/cdev/cdev.c
+++ b/drivers/staging/most/cdev/cdev.c
@@ -292,13 +292,15 @@ static __poll_t comp_poll(struct file *filp, poll_table *wait)
 
 	poll_wait(filp, &c->wq, wait);
 
+	mutex_lock(&c->io_mutex);
 	if (c->cfg->direction == MOST_CH_RX) {
-		if (!kfifo_is_empty(&c->fifo))
+		if (!c->dev || !kfifo_is_empty(&c->fifo))
 			mask |= EPOLLIN | EPOLLRDNORM;
 	} else {
-		if (!kfifo_is_empty(&c->fifo) || ch_has_mbo(c))
+		if (!c->dev || !kfifo_is_empty(&c->fifo) || ch_has_mbo(c))
 			mask |= EPOLLOUT | EPOLLWRNORM;
 	}
+	mutex_unlock(&c->io_mutex);
 	return mask;
 }
 
-- 
2.7.4

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

[PATCH 23/28] staging: most: dim2: use device tree

[Christian Gromm]

This patch removes the dependency to platform specific source files
that do platform specific initialization and supply the IRQ number.
Instead DT code is added

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/dim2/Kconfig |   2 +-
 drivers/staging/most/dim2/dim2.c  | 224 +++++++++++++++++++++++++++++++++-----
 drivers/staging/most/dim2/dim2.h  |  22 ----
 3 files changed, 196 insertions(+), 52 deletions(-)
 delete mode 100644 drivers/staging/most/dim2/dim2.h

diff --git a/drivers/staging/most/dim2/Kconfig b/drivers/staging/most/dim2/Kconfig
index e39c4e5..5aeef22 100644
--- a/drivers/staging/most/dim2/Kconfig
+++ b/drivers/staging/most/dim2/Kconfig
@@ -4,7 +4,7 @@
 
 config MOST_DIM2
 	tristate "DIM2"
-	depends on HAS_IOMEM
+	depends on HAS_IOMEM && OF
 
 	---help---
 	  Say Y here if you want to connect via MediaLB to network transceiver.
diff --git a/drivers/staging/most/dim2/dim2.c b/drivers/staging/most/dim2/dim2.c
index e60efa0..5e3accb 100644
--- a/drivers/staging/most/dim2/dim2.c
+++ b/drivers/staging/most/dim2/dim2.c
@@ -8,6 +8,7 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/module.h>
+#include <linux/of_platform.h>
 #include <linux/printk.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
@@ -15,13 +16,13 @@
 #include <linux/interrupt.h>
 #include <linux/slab.h>
 #include <linux/io.h>
+#include <linux/clk.h>
 #include <linux/dma-mapping.h>
 #include <linux/sched.h>
 #include <linux/kthread.h>
 
 #include "most/core.h"
 #include "hal.h"
-#include "dim2.h"
 #include "errors.h"
 #include "sysfs.h"
 
@@ -88,6 +89,9 @@ struct dim2_hdm {
 	struct most_interface most_iface;
 	char name[16 + sizeof "dim2-"];
 	void __iomem *io_base;
+	u8 clk_speed;
+	struct clk *clk;
+	struct clk *clk_pll;
 	struct task_struct *netinfo_task;
 	wait_queue_head_t netinfo_waitq;
 	int deliver_netinfo;
@@ -97,6 +101,12 @@ struct dim2_hdm {
 	struct medialb_bus bus;
 	void (*on_netinfo)(struct most_interface *most_iface,
 			   unsigned char link_state, unsigned char *addrs);
+	void (*disable_platform)(struct platform_device *);
+};
+
+struct dim2_platform_data {
+	int (*enable)(struct platform_device *);
+	void (*disable)(struct platform_device *);
 };
 
 #define iface_to_hdm(iface) container_of(iface, struct dim2_hdm, most_iface)
@@ -681,6 +691,8 @@ static void dma_free(struct mbo *mbo, u32 size)
 	dma_free_coherent(NULL, size, mbo->virt_address, mbo->bus_address);
 }
 
+static const struct of_device_id dim2_of_match[];
+
 /*
  * dim2_probe - dim2 probe handler
  * @pdev: platform device structure
@@ -690,13 +702,16 @@ static void dma_free(struct mbo *mbo, u32 size)
  */
 static int dim2_probe(struct platform_device *pdev)
 {
-	struct dim2_platform_data *pdata = pdev->dev.platform_data;
+	const struct dim2_platform_data *pdata;
+	const struct of_device_id *of_id;
 	struct dim2_hdm *dev;
 	struct resource *res;
 	int ret, i;
 	u8 hal_ret;
 	int irq;
 
+	enum { MLB_INT_IDX, AHB0_INT_IDX };
+
 	dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
 	if (!dev)
 		return -ENOMEM;
@@ -704,29 +719,31 @@ static int dim2_probe(struct platform_device *pdev)
 	dev->atx_idx = -1;
 
 	platform_set_drvdata(pdev, dev);
+
+	dev->clk_speed = CLK_4096FS;
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	dev->io_base = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(dev->io_base))
 		return PTR_ERR(dev->io_base);
 
-	if (!pdata) {
-		dev_err(&pdev->dev, "missing platform data\n");
-		return -EINVAL;
-	}
-
-	ret = pdata->init ? pdata->init(pdata, dev->io_base) : 0;
+	of_id = of_match_node(dim2_of_match, pdev->dev.of_node);
+	pdata = of_id->data;
+	ret = pdata && pdata->enable ? pdata->enable(pdev) : 0;
 	if (ret)
 		return ret;
 
+	dev->disable_platform = pdata ? pdata->disable : 0;
+
 	dev_info(&pdev->dev, "sync: num of frames per sub-buffer: %u\n", fcnt);
-	hal_ret = dim_startup(dev->io_base, pdata->clk_speed, fcnt);
+	hal_ret = dim_startup(dev->io_base, dev->clk_speed, fcnt);
 	if (hal_ret != DIM_NO_ERROR) {
 		dev_err(&pdev->dev, "dim_startup failed: %d\n", hal_ret);
 		ret = -ENODEV;
-		goto err_bsp_destroy;
+		goto err_disable_platform;
 	}
 
-	irq = platform_get_irq(pdev, 0);
+	irq = platform_get_irq(pdev, AHB0_INT_IDX);
 	if (irq < 0) {
 		dev_err(&pdev->dev, "failed to get ahb0_int irq: %d\n", irq);
 		ret = irq;
@@ -740,7 +757,7 @@ static int dim2_probe(struct platform_device *pdev)
 		goto err_shutdown_dim;
 	}
 
-	irq = platform_get_irq(pdev, 1);
+	irq = platform_get_irq(pdev, MLB_INT_IDX);
 	if (irq < 0) {
 		dev_err(&pdev->dev, "failed to get mlb_int irq: %d\n", irq);
 		ret = irq;
@@ -828,9 +845,9 @@ static int dim2_probe(struct platform_device *pdev)
 	kthread_stop(dev->netinfo_task);
 err_shutdown_dim:
 	dim_shutdown();
-err_bsp_destroy:
-	if (pdata && pdata->destroy)
-		pdata->destroy(pdata);
+err_disable_platform:
+	if (dev->disable_platform)
+		dev->disable_platform(pdev);
 
 	return ret;
 }
@@ -844,7 +861,6 @@ static int dim2_probe(struct platform_device *pdev)
 static int dim2_remove(struct platform_device *pdev)
 {
 	struct dim2_hdm *dev = platform_get_drvdata(pdev);
-	struct dim2_platform_data *pdata = pdev->dev.platform_data;
 	unsigned long flags;
 
 	dim2_sysfs_destroy(&dev->dev);
@@ -855,37 +871,187 @@ static int dim2_remove(struct platform_device *pdev)
 	dim_shutdown();
 	spin_unlock_irqrestore(&dim_lock, flags);
 
-	if (pdata && pdata->destroy)
-		pdata->destroy(pdata);
+	if (dev->disable_platform)
+		dev->disable_platform(pdev);
+
+	return 0;
+}
+
+/* platform specific functions [[ */
+
+static int fsl_mx6_enable(struct platform_device *pdev)
+{
+	struct dim2_hdm *dev = platform_get_drvdata(pdev);
+	int ret;
+
+	dev->clk = devm_clk_get(&pdev->dev, "mlb");
+	if (IS_ERR_OR_NULL(dev->clk)) {
+		dev_err(&pdev->dev, "unable to get mlb clock\n");
+		return -EFAULT;
+	}
+
+	ret = clk_prepare_enable(dev->clk);
+	if (ret) {
+		dev_err(&pdev->dev, "%s\n", "clk_prepare_enable failed");
+		return ret;
+	}
+
+	if (dev->clk_speed >= CLK_2048FS) {
+		/* enable pll */
+		dev->clk_pll = devm_clk_get(&pdev->dev, "pll8_mlb");
+		if (IS_ERR_OR_NULL(dev->clk_pll)) {
+			dev_err(&pdev->dev, "unable to get mlb pll clock\n");
+			clk_disable_unprepare(dev->clk);
+			return -EFAULT;
+		}
+
+		writel(0x888, dev->io_base + 0x38);
+		clk_prepare_enable(dev->clk_pll);
+	}
+
+	return 0;
+}
+
+static void fsl_mx6_disable(struct platform_device *pdev)
+{
+	struct dim2_hdm *dev = platform_get_drvdata(pdev);
+
+	if (dev->clk_speed >= CLK_2048FS)
+		clk_disable_unprepare(dev->clk_pll);
+
+	clk_disable_unprepare(dev->clk);
+}
+
+static int rcar_h2_enable(struct platform_device *pdev)
+{
+	struct dim2_hdm *dev = platform_get_drvdata(pdev);
+	int ret;
+
+	dev->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(dev->clk)) {
+		dev_err(&pdev->dev, "cannot get clock\n");
+		return PTR_ERR(dev->clk);
+	}
+
+	ret = clk_prepare_enable(dev->clk);
+	if (ret) {
+		dev_err(&pdev->dev, "%s\n", "clk_prepare_enable failed");
+		return ret;
+	}
+
+	if (dev->clk_speed >= CLK_2048FS) {
+		/* enable MLP pll and LVDS drivers */
+		writel(0x03, dev->io_base + 0x600);
+		/* set bias */
+		writel(0x888, dev->io_base + 0x38);
+	} else {
+		/* PLL */
+		writel(0x04, dev->io_base + 0x600);
+	}
+
 
-	/*
-	 * break link to local platform_device_id struct
-	 * to prevent crash by unload platform device module
-	 */
-	pdev->id_entry = NULL;
+	/* BBCR = 0b11 */
+	writel(0x03, dev->io_base + 0x500);
+	writel(0x0002FF02, dev->io_base + 0x508);
 
 	return 0;
 }
 
-static const struct platform_device_id dim2_id[] = {
-	{ "medialb_dim2" },
-	{ }, /* Terminating entry */
+static void rcar_h2_disable(struct platform_device *pdev)
+{
+	struct dim2_hdm *dev = platform_get_drvdata(pdev);
+
+	clk_disable_unprepare(dev->clk);
+
+	/* disable PLLs and LVDS drivers */
+	writel(0x0, dev->io_base + 0x600);
+}
+
+static int rcar_m3_enable(struct platform_device *pdev)
+{
+	struct dim2_hdm *dev = platform_get_drvdata(pdev);
+	u32 enable_512fs = dev->clk_speed == CLK_512FS;
+	int ret;
+
+	dev->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(dev->clk)) {
+		dev_err(&pdev->dev, "cannot get clock\n");
+		return PTR_ERR(dev->clk);
+	}
+
+	ret = clk_prepare_enable(dev->clk);
+	if (ret) {
+		dev_err(&pdev->dev, "%s\n", "clk_prepare_enable failed");
+		return ret;
+	}
+
+	/* PLL */
+	writel(0x04, dev->io_base + 0x600);
+
+	writel(enable_512fs, dev->io_base + 0x604);
+
+	/* BBCR = 0b11 */
+	writel(0x03, dev->io_base + 0x500);
+	writel(0x0002FF02, dev->io_base + 0x508);
+
+	return 0;
+}
+
+static void rcar_m3_disable(struct platform_device *pdev)
+{
+	struct dim2_hdm *dev = platform_get_drvdata(pdev);
+
+	clk_disable_unprepare(dev->clk);
+
+	/* disable PLLs and LVDS drivers */
+	writel(0x0, dev->io_base + 0x600);
+}
+
+/* ]] platform specific functions */
+
+enum dim2_platforms { FSL_MX6, RCAR_H2, RCAR_M3 };
+
+static struct dim2_platform_data plat_data[] = {
+	[FSL_MX6] = { .enable = fsl_mx6_enable, .disable = fsl_mx6_disable },
+	[RCAR_H2] = { .enable = rcar_h2_enable, .disable = rcar_h2_disable },
+	[RCAR_M3] = { .enable = rcar_m3_enable, .disable = rcar_m3_disable },
+};
+
+static const struct of_device_id dim2_of_match[] = {
+	{
+		.compatible = "fsl,imx6q-mlb150",
+		.data = plat_data + FSL_MX6
+	},
+	{
+		.compatible = "renesas,mlp",
+		.data = plat_data + RCAR_H2
+	},
+	{
+		.compatible = "rcar,medialb-dim2",
+		.data = plat_data + RCAR_M3
+	},
+	{
+		.compatible = "xlnx,axi4-os62420_3pin-1.00.a",
+	},
+	{
+		.compatible = "xlnx,axi4-os62420_6pin-1.00.a",
+	},
+	{},
 };
 
-MODULE_DEVICE_TABLE(platform, dim2_id);
+MODULE_DEVICE_TABLE(of, dim2_of_match);
 
 static struct platform_driver dim2_driver = {
 	.probe = dim2_probe,
 	.remove = dim2_remove,
-	.id_table = dim2_id,
 	.driver = {
 		.name = "hdm_dim2",
+		.of_match_table = dim2_of_match,
 	},
 };
 
 module_platform_driver(dim2_driver);
 
-MODULE_AUTHOR("Jain Roy Ambi <JainRoy.Ambi@microchip.com>");
 MODULE_AUTHOR("Andrey Shvetsov <andrey.shvetsov@k2l.de>");
 MODULE_DESCRIPTION("MediaLB DIM2 Hardware Dependent Module");
 MODULE_LICENSE("GPL");
diff --git a/drivers/staging/most/dim2/dim2.h b/drivers/staging/most/dim2/dim2.h
deleted file mode 100644
index 999d4d4..0000000
--- a/drivers/staging/most/dim2/dim2.h
+++ /dev/null
@@ -1,22 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * dim2.h - MediaLB DIM2 HDM Header
- *
- * Copyright (C) 2015, Microchip Technology Germany II GmbH & Co. KG
- */
-
-#ifndef DIM2_HDM_H
-#define	DIM2_HDM_H
-
-#include <linux/types.h>
-
-struct device;
-
-/* platform dependent data for dim2 interface */
-struct dim2_platform_data {
-	int (*init)(struct dim2_platform_data *pd, void __iomem *io_base);
-	void (*destroy)(struct dim2_platform_data *pd);
-	u8 clk_speed;
-};
-
-#endif	/* DIM2_HDM_H */
-- 
2.7.4

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

Re: [PATCH 23/28] staging: most: dim2: use device tree

[Geert Uytterhoeven]

 	Hi Christian, Greg,

CC devicetree, linux-renesas-soc

On Tue, 8 May 2018, Christian Gromm wrote:
> This patch removes the dependency to platform specific source files
> that do platform specific initialization and supply the IRQ number.
> Instead DT code is added
>
> Signed-off-by: Christian Gromm <christian.gromm@microchip.com>

This patch bypassed review by the DT people, and ended up in v4.18 as
commit 21e57ff086056c01 ("staging: most: dim2: use device tree").

> --- a/drivers/staging/most/dim2/dim2.c
> +++ b/drivers/staging/most/dim2/dim2.c
> +static const struct of_device_id dim2_of_match[] = {
> +	{
> +		.compatible = "fsl,imx6q-mlb150",
> +		.data = plat_data + FSL_MX6
> +	},
> +	{
> +		.compatible = "renesas,mlp",
> +		.data = plat_data + RCAR_H2
> +	},
> +	{
> +		.compatible = "rcar,medialb-dim2",
> +		.data = plat_data + RCAR_M3
> +	},
> +	{
> +		.compatible = "xlnx,axi4-os62420_3pin-1.00.a",
> +	},
> +	{
> +		.compatible = "xlnx,axi4-os62420_6pin-1.00.a",
> +	},
> +	{},
> };

There are no documented DT bindings for this hardware block, nor any
upstream example users.  Given some compatible values do not follow
standard practises (no idea about the other parts), it's very likely
these de facto bindings, and all their out-of-tree users, will have to
be changed.

Gr{oetje,eeting}s,

 						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
 							    -- Linus Torvalds

Re: [PATCH 23/28] staging: most: dim2: use device tree

[Greg KH]

On Tue, Oct 12, 2021 at 08:14:14PM +0200, Geert Uytterhoeven wrote:
> 	Hi Christian, Greg,
> 
> CC devicetree, linux-renesas-soc
> 
> On Tue, 8 May 2018, Christian Gromm wrote:
> > This patch removes the dependency to platform specific source files
> > that do platform specific initialization and supply the IRQ number.
> > Instead DT code is added
> > 
> > Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> 
> This patch bypassed review by the DT people, and ended up in v4.18 as
> commit 21e57ff086056c01 ("staging: most: dim2: use device tree").

Yeah, staging-only dt changes are not usually run by the dt maintainers.

> 
> > --- a/drivers/staging/most/dim2/dim2.c
> > +++ b/drivers/staging/most/dim2/dim2.c
> > +static const struct of_device_id dim2_of_match[] = {
> > +	{
> > +		.compatible = "fsl,imx6q-mlb150",
> > +		.data = plat_data + FSL_MX6
> > +	},
> > +	{
> > +		.compatible = "renesas,mlp",
> > +		.data = plat_data + RCAR_H2
> > +	},
> > +	{
> > +		.compatible = "rcar,medialb-dim2",
> > +		.data = plat_data + RCAR_M3
> > +	},
> > +	{
> > +		.compatible = "xlnx,axi4-os62420_3pin-1.00.a",
> > +	},
> > +	{
> > +		.compatible = "xlnx,axi4-os62420_6pin-1.00.a",
> > +	},
> > +	{},
> > };
> 
> There are no documented DT bindings for this hardware block, nor any
> upstream example users.  Given some compatible values do not follow
> standard practises (no idea about the other parts), it's very likely
> these de facto bindings, and all their out-of-tree users, will have to
> be changed.

Great, fix the bindings and anything in-kernel here please.  We don't
care about out-of-kernel stuff for drivers/staging/ at all.

thanks,

greg k-h

[PATCH 24/28] staging: most: dim2: read clock speed from the device

[Christian Gromm]

This patch implemets reading of the clock speed from DT.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/dim2/dim2.c | 50 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/most/dim2/dim2.c b/drivers/staging/most/dim2/dim2.c
index 5e3accb..25e6e7e 100644
--- a/drivers/staging/most/dim2/dim2.c
+++ b/drivers/staging/most/dim2/dim2.c
@@ -693,6 +693,42 @@ static void dma_free(struct mbo *mbo, u32 size)
 
 static const struct of_device_id dim2_of_match[];
 
+static struct {
+	const char *clock_speed;
+	u8 clk_speed;
+} clk_mt[] = {
+	{ "256fs", CLK_256FS },
+	{ "512fs", CLK_512FS },
+	{ "1024fs", CLK_1024FS },
+	{ "2048fs", CLK_2048FS },
+	{ "3072fs", CLK_3072FS },
+	{ "4096fs", CLK_4096FS },
+	{ "6144fs", CLK_6144FS },
+	{ "8192fs", CLK_8192FS },
+};
+
+/**
+ * get_dim2_clk_speed - converts string to DIM2 clock speed value
+ *
+ * @clock_speed: string in the format "{NUMBER}fs"
+ * @val: pointer to get one of the CLK_{NUMBER}FS values
+ *
+ * By success stores one of the CLK_{NUMBER}FS in the *val and returns 0,
+ * otherwise returns -EINVAL.
+ */
+static int get_dim2_clk_speed(const char *clock_speed, u8 *val)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(clk_mt); i++) {
+		if (!strcmp(clock_speed, clk_mt[i].clock_speed)) {
+			*val = clk_mt[i].clk_speed;
+			return 0;
+		}
+	}
+	return -EINVAL;
+}
+
 /*
  * dim2_probe - dim2 probe handler
  * @pdev: platform device structure
@@ -704,6 +740,7 @@ static int dim2_probe(struct platform_device *pdev)
 {
 	const struct dim2_platform_data *pdata;
 	const struct of_device_id *of_id;
+	const char *clock_speed;
 	struct dim2_hdm *dev;
 	struct resource *res;
 	int ret, i;
@@ -720,7 +757,18 @@ static int dim2_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, dev);
 
-	dev->clk_speed = CLK_4096FS;
+	ret = of_property_read_string(pdev->dev.of_node,
+				      "microchip,clock-speed", &clock_speed);
+	if (ret) {
+		dev_err(&pdev->dev, "missing dt property clock-speed\n");
+		return ret;
+	}
+
+	ret = get_dim2_clk_speed(clock_speed, &dev->clk_speed);
+	if (ret) {
+		dev_err(&pdev->dev, "bad dt property clock-speed\n");
+		return ret;
+	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	dev->io_base = devm_ioremap_resource(&pdev->dev, res);
-- 
2.7.4

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

[PATCH 25/28] staging: most: dim2: use device to allocate coherent memory

[Christian Gromm]

On several architectures the allocation of coherent memory needs a device
that has the dma_ops structure properly initialized. This patch enables
the DIM2 platform to be used to allocate this type of memory.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/dim2/dim2.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/most/dim2/dim2.c b/drivers/staging/most/dim2/dim2.c
index 25e6e7e..fe90a7c 100644
--- a/drivers/staging/most/dim2/dim2.c
+++ b/drivers/staging/most/dim2/dim2.c
@@ -683,12 +683,16 @@ static int poison_channel(struct most_interface *most_iface, int ch_idx)
 
 static void *dma_alloc(struct mbo *mbo, u32 size)
 {
-	return dma_alloc_coherent(NULL, size, &mbo->bus_address, GFP_KERNEL);
+	struct device *dev = mbo->ifp->driver_dev;
+
+	return dma_alloc_coherent(dev, size, &mbo->bus_address, GFP_KERNEL);
 }
 
 static void dma_free(struct mbo *mbo, u32 size)
 {
-	dma_free_coherent(NULL, size, mbo->virt_address, mbo->bus_address);
+	struct device *dev = mbo->ifp->driver_dev;
+
+	dma_free_coherent(dev, size, mbo->virt_address, mbo->bus_address);
 }
 
 static const struct of_device_id dim2_of_match[];
@@ -870,6 +874,7 @@ static int dim2_probe(struct platform_device *pdev)
 	dev->most_iface.dma_free = dma_free;
 	dev->most_iface.poison_channel = poison_channel;
 	dev->most_iface.request_netinfo = request_netinfo;
+	dev->most_iface.driver_dev = &pdev->dev;
 	dev->dev.init_name = "dim2_state";
 	dev->dev.parent = &dev->most_iface.dev;
 
-- 
2.7.4

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

[PATCH 26/28] staging: most: usb: don't set URB_ZERO_PACKET flag for synchronous data

[Christian Gromm]

This patch avoids setting the URB_ZERO_PACKET transfer flag for synchronous
data. This is needed to prevent the host from sending an empty packet when
data is aligned to an endpoint packet boundary.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/usb/usb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/most/usb/usb.c b/drivers/staging/most/usb/usb.c
index 3126b69..d102b08 100644
--- a/drivers/staging/most/usb/usb.c
+++ b/drivers/staging/most/usb/usb.c
@@ -592,7 +592,8 @@ static int hdm_enqueue(struct most_interface *iface, int channel,
 				  length,
 				  hdm_write_completion,
 				  mbo);
-		if (conf->data_type != MOST_CH_ISOC)
+		if (conf->data_type != MOST_CH_ISOC &&
+		    conf->data_type != MOST_CH_SYNC)
 			urb->transfer_flags |= URB_ZERO_PACKET;
 	} else {
 		usb_fill_bulk_urb(urb, mdev->usb_device,
-- 
2.7.4

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

[PATCH 27/28] staging: most: usb: fix usb_disconnect race condition

[Christian Gromm]

The functions usb_disconnect and usb_sndbulkpipe are racing for the struct
usb_device, which might cause a null pointer dereference exception. This
patch fixes this race condition by protecting the critical section inside
the function hdm_enque with the io_mutex.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/usb/usb.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/most/usb/usb.c b/drivers/staging/most/usb/usb.c
index d102b08..19ad618 100644
--- a/drivers/staging/most/usb/usb.c
+++ b/drivers/staging/most/usb/usb.c
@@ -565,14 +565,19 @@ static int hdm_enqueue(struct most_interface *iface, int channel,
 
 	mdev = to_mdev(iface);
 	conf = &mdev->conf[channel];
-	dev = &mdev->usb_device->dev;
 
-	if (!mdev->usb_device)
-		return -ENODEV;
+	mutex_lock(&mdev->io_mutex);
+	if (!mdev->usb_device) {
+		retval = -ENODEV;
+		goto _exit;
+	}
 
+	dev = &mdev->usb_device->dev;
 	urb = usb_alloc_urb(NO_ISOCHRONOUS_URB, GFP_ATOMIC);
-	if (!urb)
-		return -ENOMEM;
+	if (!urb) {
+		retval = -ENOMEM;
+		goto _exit;
+	}
 
 	if ((conf->direction & MOST_CH_TX) && mdev->padding_active[channel] &&
 	    hdm_add_padding(mdev, channel, mbo)) {
@@ -613,12 +618,14 @@ static int hdm_enqueue(struct most_interface *iface, int channel,
 		dev_err(dev, "URB submit failed with error %d.\n", retval);
 		goto _error_1;
 	}
-	return 0;
+	goto _exit;
 
 _error_1:
 	usb_unanchor_urb(urb);
 _error:
 	usb_free_urb(urb);
+_exit:
+	mutex_unlock(&mdev->io_mutex);
 	return retval;
 }
 
-- 
2.7.4

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

[PATCH 28/28] staging: most: usb: remove local variable

[Christian Gromm]

This patch removes the local variable dev that is used to store the pointer
to the usb_device whenever it is used only once.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/usb/usb.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/most/usb/usb.c b/drivers/staging/most/usb/usb.c
index 19ad618..bc820f9 100644
--- a/drivers/staging/most/usb/usb.c
+++ b/drivers/staging/most/usb/usb.c
@@ -338,7 +338,6 @@ static void hdm_write_completion(struct urb *urb)
 	struct mbo *mbo = urb->context;
 	struct most_dev *mdev = to_mdev(mbo->ifp);
 	unsigned int channel = mbo->hdm_channel_id;
-	struct device *dev = &mdev->usb_device->dev;
 	spinlock_t *lock = mdev->channel_lock + channel;
 	unsigned long flags;
 
@@ -354,7 +353,8 @@ static void hdm_write_completion(struct urb *urb)
 			mbo->status = MBO_SUCCESS;
 			break;
 		case -EPIPE:
-			dev_warn(dev, "Broken pipe on ep%02x\n",
+			dev_warn(&mdev->usb_device->dev,
+				 "Broken pipe on ep%02x\n",
 				 mdev->ep_address[channel]);
 			mdev->is_channel_healthy[channel] = false;
 			mdev->clear_work[channel].pipe = urb->pipe;
@@ -552,7 +552,6 @@ static int hdm_enqueue(struct most_interface *iface, int channel,
 {
 	struct most_dev *mdev;
 	struct most_channel_config *conf;
-	struct device *dev;
 	int retval = 0;
 	struct urb *urb;
 	unsigned long length;
@@ -572,7 +571,6 @@ static int hdm_enqueue(struct most_interface *iface, int channel,
 		goto _exit;
 	}
 
-	dev = &mdev->usb_device->dev;
 	urb = usb_alloc_urb(NO_ISOCHRONOUS_URB, GFP_ATOMIC);
 	if (!urb) {
 		retval = -ENOMEM;
@@ -615,7 +613,8 @@ static int hdm_enqueue(struct most_interface *iface, int channel,
 
 	retval = usb_submit_urb(urb, GFP_KERNEL);
 	if (retval) {
-		dev_err(dev, "URB submit failed with error %d.\n", retval);
+		dev_err(&mdev->usb_device->dev,
+			"URB submit failed with error %d.\n", retval);
 		goto _error_1;
 	}
 	goto _exit;
-- 
2.7.4

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

Re: [PATCH 16/28] staging: most: sound: call snd_card_new with struct device

[Eugeniu Rosca]

Hi Christian,

On Tue, 08 May 2018 02:46:44 -0700, Christian Gromm wrote:
>     This patch is needed as function snd_card_new needs a valid
>     parent device. Passing a NULL pointer leads to kernel Ooops.
>     
>     Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> ---
>  drivers/staging/most/core.h        | 1 +
>  drivers/staging/most/sound/sound.c | 2 +-
>  drivers/staging/most/usb/usb.c     | 1 +
>  3 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/most/core.h b/drivers/staging/most/core.h
> index 7a3c70bead19..64cc02f161e7 100644
> --- a/drivers/staging/most/core.h
> +++ b/drivers/staging/most/core.h
> @@ -230,6 +230,7 @@ struct mbo {
>   */
>  struct most_interface {
>  	struct device dev;
> +	struct device *driver_dev;
>  	struct module *mod;
>  	enum most_interface_type interface;
>  	const char *description;
> diff --git a/drivers/staging/most/sound/sound.c b/drivers/staging/most/sound/sound.c
> index 18f722410a63..04c18323c2ea 100644
> --- a/drivers/staging/most/sound/sound.c
> +++ b/drivers/staging/most/sound/sound.c
> @@ -590,7 +590,7 @@ static int audio_probe_channel(struct most_interface *iface, int channel_id,
>  	if (ret < 0)
>  		return ret;
>  
> -	ret = snd_card_new(NULL, -1, card_name, THIS_MODULE,
> +	ret = snd_card_new(&iface->dev, -1, card_name, THIS_MODULE,
>  			   sizeof(*channel), &card);
>  	if (ret < 0)
>  		return ret;
> diff --git a/drivers/staging/most/usb/usb.c b/drivers/staging/most/usb/usb.c
> index 5ed1dccc0839..f18726049528 100644
> --- a/drivers/staging/most/usb/usb.c
> +++ b/drivers/staging/most/usb/usb.c
> @@ -1043,6 +1043,7 @@ hdm_probe(struct usb_interface *interface, const struct usb_device_id *id)
>  	mdev->link_stat_timer.expires = jiffies + (2 * HZ);
>  
>  	mdev->iface.mod = hdm_usb_fops.owner;
> +	mdev->iface.driver_dev = &interface->dev;
>  	mdev->iface.interface = ITYPE_USB;
>  	mdev->iface.configure = hdm_configure_channel;
>  	mdev->iface.request_netinfo = hdm_request_netinfo;

Just for your information, when mapping commits from vanilla to those
from https://github.com/microchip-ais/linux/commits/mld-1.8.0, we've
stumbled upon some subtle but striking difference between mld-1.8.0
commit [0] and v4.18-rc1 commit [1]. The latter looks like an upstreamed
version of the former. However, while commit [0] creates a new 'dev'
member in 'struct most_interface' and uses it consistently, commit [1]
creates 'driver_dev' and uses it intermixed with 'dev'.

Since we don't use aim-sound, we just signal this feedback to you
as FWIW without sending a patch (which we can't test).

[0] https://github.com/microchip-ais/linux/commit/2fef0f89f04703
    ("staging: most: add struct device to most interface")
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=69c90cf1b2faf5
    ("staging: most: sound: call snd_card_new with struct device")

-- 
Best regards,
Eugeniu.

Re: [PATCH 16/28] staging: most: sound: call snd_card_new with struct device

[Eugeniu Rosca]

Hi Christian,

On Tue, 08 May 2018 02:46:44 -0700, Christian Gromm wrote:
>     This patch is needed as function snd_card_new needs a valid
>     parent device. Passing a NULL pointer leads to kernel Ooops.
>     
>     Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> ---
>  drivers/staging/most/core.h        | 1 +
>  drivers/staging/most/sound/sound.c | 2 +-
>  drivers/staging/most/usb/usb.c     | 1 +
>  3 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/most/core.h b/drivers/staging/most/core.h
> index 7a3c70bead19..64cc02f161e7 100644
> --- a/drivers/staging/most/core.h
> +++ b/drivers/staging/most/core.h
> @@ -230,6 +230,7 @@ struct mbo {
>   */
>  struct most_interface {
>  	struct device dev;
> +	struct device *driver_dev;
>  	struct module *mod;
>  	enum most_interface_type interface;
>  	const char *description;
> diff --git a/drivers/staging/most/sound/sound.c b/drivers/staging/most/sound/sound.c
> index 18f722410a63..04c18323c2ea 100644
> --- a/drivers/staging/most/sound/sound.c
> +++ b/drivers/staging/most/sound/sound.c
> @@ -590,7 +590,7 @@ static int audio_probe_channel(struct most_interface *iface, int channel_id,
>  	if (ret < 0)
>  		return ret;
>  
> -	ret = snd_card_new(NULL, -1, card_name, THIS_MODULE,
> +	ret = snd_card_new(&iface->dev, -1, card_name, THIS_MODULE,
>  			   sizeof(*channel), &card);
>  	if (ret < 0)
>  		return ret;
> diff --git a/drivers/staging/most/usb/usb.c b/drivers/staging/most/usb/usb.c
> index 5ed1dccc0839..f18726049528 100644
> --- a/drivers/staging/most/usb/usb.c
> +++ b/drivers/staging/most/usb/usb.c
> @@ -1043,6 +1043,7 @@ hdm_probe(struct usb_interface *interface, const struct usb_device_id *id)
>  	mdev->link_stat_timer.expires = jiffies + (2 * HZ);
>  
>  	mdev->iface.mod = hdm_usb_fops.owner;
> +	mdev->iface.driver_dev = &interface->dev;
>  	mdev->iface.interface = ITYPE_USB;
>  	mdev->iface.configure = hdm_configure_channel;
>  	mdev->iface.request_netinfo = hdm_request_netinfo;

Just for your information, when mapping commits from vanilla to those
from https://github.com/microchip-ais/linux/commits/mld-1.8.0, we've
stumbled upon some subtle but striking difference between mld-1.8.0
commit [0] and v4.18-rc1 commit [1]. The latter looks like an upstreamed
version of the former. However, while commit [0] creates a new 'dev'
member in 'struct most_interface' and uses it consistently, commit [1]
creates 'driver_dev' and uses it intermixed with 'dev'.

Since we don't use aim-sound, we just signal this feedback to you
as FWIW without sending a patch (which we can't test).

[0] https://github.com/microchip-ais/linux/commit/2fef0f89f04703
    ("staging: most: add struct device to most interface")
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=69c90cf1b2faf5
    ("staging: most: sound: call snd_card_new with struct device")

-- 
Best regards,
Eugeniu.

Re: [PATCH 16/28] staging: most: sound: call snd_card_new with struct device

[Christian.Gromm]

On Mi, 2019-04-24 at 20:50 +0200, Eugeniu Rosca wrote:
> External E-Mail
> 
> 
> Hi Christian,
> 
> On Tue, 08 May 2018 02:46:44 -0700, Christian Gromm wrote:
> > 
> >     This patch is needed as function snd_card_new needs a valid
> >     parent device. Passing a NULL pointer leads to kernel Ooops.
> >     
> >     Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> > ---
> >  drivers/staging/most/core.h        | 1 +
> >  drivers/staging/most/sound/sound.c | 2 +-
> >  drivers/staging/most/usb/usb.c     | 1 +
> >  3 files changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/most/core.h
> > b/drivers/staging/most/core.h
> > index 7a3c70bead19..64cc02f161e7 100644
> > --- a/drivers/staging/most/core.h
> > +++ b/drivers/staging/most/core.h
> > @@ -230,6 +230,7 @@ struct mbo {
> >   */
> >  struct most_interface {
> >  	struct device dev;
> > +	struct device *driver_dev;
> >  	struct module *mod;
> >  	enum most_interface_type interface;
> >  	const char *description;
> > diff --git a/drivers/staging/most/sound/sound.c
> > b/drivers/staging/most/sound/sound.c
> > index 18f722410a63..04c18323c2ea 100644
> > --- a/drivers/staging/most/sound/sound.c
> > +++ b/drivers/staging/most/sound/sound.c
> > @@ -590,7 +590,7 @@ static int audio_probe_channel(struct
> > most_interface *iface, int channel_id,
> >  	if (ret < 0)
> >  		return ret;
> >  
> > -	ret = snd_card_new(NULL, -1, card_name, THIS_MODULE,
> > +	ret = snd_card_new(&iface->dev, -1, card_name,
> > THIS_MODULE,
> >  			   sizeof(*channel), &card);
> >  	if (ret < 0)
> >  		return ret;
> > diff --git a/drivers/staging/most/usb/usb.c
> > b/drivers/staging/most/usb/usb.c
> > index 5ed1dccc0839..f18726049528 100644
> > --- a/drivers/staging/most/usb/usb.c
> > +++ b/drivers/staging/most/usb/usb.c
> > @@ -1043,6 +1043,7 @@ hdm_probe(struct usb_interface *interface,
> > const struct usb_device_id *id)
> >  	mdev->link_stat_timer.expires = jiffies + (2 * HZ);
> >  
> >  	mdev->iface.mod = hdm_usb_fops.owner;
> > +	mdev->iface.driver_dev = &interface->dev;
> >  	mdev->iface.interface = ITYPE_USB;
> >  	mdev->iface.configure = hdm_configure_channel;
> >  	mdev->iface.request_netinfo = hdm_request_netinfo;
> Just for your information, when mapping commits from vanilla to those
> from https://github.com/microchip-ais/linux/commits/mld-1.8.0, we've
> stumbled upon some subtle but striking difference between mld-1.8.0
> commit [0] and v4.18-rc1 commit [1]. The latter looks like an
> upstreamed
> version of the former. However, while commit [0] creates a new 'dev'
> member in 'struct most_interface' and uses it consistently, commit
> [1]
> creates 'driver_dev' and uses it intermixed with 'dev'.

This looks sort of suspicious. I'll check it.

Thanks for letting me know.

Regards,
Chris

> 
> Since we don't use aim-sound, we just signal this feedback to you
> as FWIW without sending a patch (which we can't test).
> 
> [0] https://github.com/microchip-ais/linux/commit/2fef0f89f04703
>     ("staging: most: add struct device to most interface")
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.gi
> t/commit/?id=69c90cf1b2faf5
>     ("staging: most: sound: call snd_card_new with struct device")
> 

Re: [PATCH 16/28] staging: most: sound: call snd_card_new with struct device

[Christian.Gromm]

On Mi, 2019-04-24 at 20:50 +0200, Eugeniu Rosca wrote:
> External E-Mail
> 
> 
> Hi Christian,
> 
> On Tue, 08 May 2018 02:46:44 -0700, Christian Gromm wrote:
> > 
> >     This patch is needed as function snd_card_new needs a valid
> >     parent device. Passing a NULL pointer leads to kernel Ooops.
> >     
> >     Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> > ---
> >  drivers/staging/most/core.h        | 1 +
> >  drivers/staging/most/sound/sound.c | 2 +-
> >  drivers/staging/most/usb/usb.c     | 1 +
> >  3 files changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/most/core.h
> > b/drivers/staging/most/core.h
> > index 7a3c70bead19..64cc02f161e7 100644
> > --- a/drivers/staging/most/core.h
> > +++ b/drivers/staging/most/core.h
> > @@ -230,6 +230,7 @@ struct mbo {
> >   */
> >  struct most_interface {
> >  	struct device dev;
> > +	struct device *driver_dev;
> >  	struct module *mod;
> >  	enum most_interface_type interface;
> >  	const char *description;
> > diff --git a/drivers/staging/most/sound/sound.c
> > b/drivers/staging/most/sound/sound.c
> > index 18f722410a63..04c18323c2ea 100644
> > --- a/drivers/staging/most/sound/sound.c
> > +++ b/drivers/staging/most/sound/sound.c
> > @@ -590,7 +590,7 @@ static int audio_probe_channel(struct
> > most_interface *iface, int channel_id,
> >  	if (ret < 0)
> >  		return ret;
> >  
> > -	ret = snd_card_new(NULL, -1, card_name, THIS_MODULE,
> > +	ret = snd_card_new(&iface->dev, -1, card_name,
> > THIS_MODULE,
> >  			   sizeof(*channel), &card);
> >  	if (ret < 0)
> >  		return ret;
> > diff --git a/drivers/staging/most/usb/usb.c
> > b/drivers/staging/most/usb/usb.c
> > index 5ed1dccc0839..f18726049528 100644
> > --- a/drivers/staging/most/usb/usb.c
> > +++ b/drivers/staging/most/usb/usb.c
> > @@ -1043,6 +1043,7 @@ hdm_probe(struct usb_interface *interface,
> > const struct usb_device_id *id)
> >  	mdev->link_stat_timer.expires = jiffies + (2 * HZ);
> >  
> >  	mdev->iface.mod = hdm_usb_fops.owner;
> > +	mdev->iface.driver_dev = &interface->dev;
> >  	mdev->iface.interface = ITYPE_USB;
> >  	mdev->iface.configure = hdm_configure_channel;
> >  	mdev->iface.request_netinfo = hdm_request_netinfo;
> Just for your information, when mapping commits from vanilla to those
> from https://github.com/microchip-ais/linux/commits/mld-1.8.0, we've
> stumbled upon some subtle but striking difference between mld-1.8.0
> commit [0] and v4.18-rc1 commit [1]. The latter looks like an
> upstreamed
> version of the former. However, while commit [0] creates a new 'dev'
> member in 'struct most_interface' and uses it consistently, commit
> [1]
> creates 'driver_dev' and uses it intermixed with 'dev'.

This looks sort of suspicious. I'll check it.

Thanks for letting me know.

Regards,
Chris

> 
> Since we don't use aim-sound, we just signal this feedback to you
> as FWIW without sending a patch (which we can't test).
> 
> [0] https://github.com/microchip-ais/linux/commit/2fef0f89f04703
>     ("staging: most: add struct device to most interface")
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.gi
> t/commit/?id=69c90cf1b2faf5
>     ("staging: most: sound: call snd_card_new with struct device")
> 
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel