2018-10-02 23:12:09

by Roman Kiryanov

[permalink] [raw]
Subject: [PATCH v2 01/15] platform: goldfish: pipe: Move the file-scope goldfish_interrupt_tasklet variable into the driver state

From: Roman Kiryanov <[email protected]>

This is a series of patches to move mutable file-scope variables
into the driver state. This change will help to introduce another
version of the pipe driver (with different state) for the older
host interface or having several instances of this device.

Signed-off-by: Roman Kiryanov <[email protected]>
---
Changes in v2:
- Updated the commit message.

drivers/platform/goldfish/goldfish_pipe.c | 24 +++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c
index 56665e879e5a..ba9aede17d57 100644
--- a/drivers/platform/goldfish/goldfish_pipe.c
+++ b/drivers/platform/goldfish/goldfish_pipe.c
@@ -208,6 +208,9 @@ struct goldfish_pipe_dev {
int irq;
int version;
unsigned char __iomem *base;
+
+ /* an irq tasklet to run goldfish_interrupt_task */
+ struct tasklet_struct irq_tasklet;
};

static struct goldfish_pipe_dev goldfish_pipe_dev;
@@ -582,14 +585,14 @@ static struct goldfish_pipe *signalled_pipes_pop_front(
return pipe;
}

-static void goldfish_interrupt_task(unsigned long unused)
+static void goldfish_interrupt_task(unsigned long dev_addr)
{
/* Iterate over the signalled pipes and wake them one by one */
+ struct goldfish_pipe_dev *dev = (struct goldfish_pipe_dev *)dev_addr;
struct goldfish_pipe *pipe;
int wakes;

- while ((pipe = signalled_pipes_pop_front(&goldfish_pipe_dev, &wakes)) !=
- NULL) {
+ while ((pipe = signalled_pipes_pop_front(dev, &wakes)) != NULL) {
if (wakes & PIPE_WAKE_CLOSED) {
pipe->flags = 1 << BIT_CLOSED_ON_HOST;
} else {
@@ -605,7 +608,6 @@ static void goldfish_interrupt_task(unsigned long unused)
wake_up_interruptible(&pipe->wake_queue);
}
}
-static DECLARE_TASKLET(goldfish_interrupt_tasklet, goldfish_interrupt_task, 0);

/*
* The general idea of the interrupt handling:
@@ -648,7 +650,7 @@ static irqreturn_t goldfish_pipe_interrupt(int irq, void *dev_id)

spin_unlock_irqrestore(&dev->lock, flags);

- tasklet_schedule(&goldfish_interrupt_tasklet);
+ tasklet_schedule(&dev->irq_tasklet);
return IRQ_HANDLED;
}

@@ -800,9 +802,14 @@ static void write_pa_addr(void *addr, void __iomem *portl, void __iomem *porth)
static int goldfish_pipe_device_init(struct platform_device *pdev)
{
struct goldfish_pipe_dev *dev = &goldfish_pipe_dev;
- int err = devm_request_irq(&pdev->dev, dev->irq,
- goldfish_pipe_interrupt,
- IRQF_SHARED, "goldfish_pipe", dev);
+ int err;
+
+ tasklet_init(&dev->irq_tasklet, &goldfish_interrupt_task,
+ (unsigned long)dev);
+
+ err = devm_request_irq(&pdev->dev, dev->irq,
+ goldfish_pipe_interrupt,
+ IRQF_SHARED, "goldfish_pipe", dev);
if (err) {
dev_err(&pdev->dev, "unable to allocate IRQ for v2\n");
return err;
@@ -854,6 +861,7 @@ static int goldfish_pipe_device_init(struct platform_device *pdev)
static void goldfish_pipe_device_deinit(struct platform_device *pdev)
{
misc_deregister(&goldfish_pipe_miscdev);
+ tasklet_kill(&goldfish_pipe_dev.irq_tasklet);
kfree(goldfish_pipe_dev.pipes);
free_page((unsigned long)goldfish_pipe_dev.buffers);
}
--
2.19.0.605.g01d371f741-goog



2018-10-02 23:11:07

by Roman Kiryanov

[permalink] [raw]
Subject: [PATCH v2 02/15] platform: goldfish: pipe: Move the file-scope goldfish_pipe_miscdev variable into the driver state

From: Roman Kiryanov <[email protected]>

This is a series of patches to move mutable file-scope variables
into the driver state. This change will help to introduce another
version of the pipe driver (with different state) for the older
host interface or having several instances of this device.

Signed-off-by: Roman Kiryanov <[email protected]>
---
Changes in v2:
- Updated the commit message.

drivers/platform/goldfish/goldfish_pipe.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c
index ba9aede17d57..8ca709b45e1f 100644
--- a/drivers/platform/goldfish/goldfish_pipe.c
+++ b/drivers/platform/goldfish/goldfish_pipe.c
@@ -211,6 +211,8 @@ struct goldfish_pipe_dev {

/* an irq tasklet to run goldfish_interrupt_task */
struct tasklet_struct irq_tasklet;
+
+ struct miscdevice miscdev;
};

static struct goldfish_pipe_dev goldfish_pipe_dev;
@@ -785,11 +787,14 @@ static const struct file_operations goldfish_pipe_fops = {
.release = goldfish_pipe_release,
};

-static struct miscdevice goldfish_pipe_miscdev = {
- .minor = MISC_DYNAMIC_MINOR,
- .name = "goldfish_pipe",
- .fops = &goldfish_pipe_fops,
-};
+static void init_miscdevice(struct miscdevice *miscdev)
+{
+ memset(miscdev, 0, sizeof(*miscdev));
+
+ miscdev->minor = MISC_DYNAMIC_MINOR;
+ miscdev->name = "goldfish_pipe";
+ miscdev->fops = &goldfish_pipe_fops;
+}

static void write_pa_addr(void *addr, void __iomem *portl, void __iomem *porth)
{
@@ -815,7 +820,8 @@ static int goldfish_pipe_device_init(struct platform_device *pdev)
return err;
}

- err = misc_register(&goldfish_pipe_miscdev);
+ init_miscdevice(&dev->miscdev);
+ err = misc_register(&dev->miscdev);
if (err) {
dev_err(&pdev->dev, "unable to register v2 device\n");
return err;
@@ -860,7 +866,7 @@ static int goldfish_pipe_device_init(struct platform_device *pdev)

static void goldfish_pipe_device_deinit(struct platform_device *pdev)
{
- misc_deregister(&goldfish_pipe_miscdev);
+ misc_deregister(&goldfish_pipe_dev.miscdev);
tasklet_kill(&goldfish_pipe_dev.irq_tasklet);
kfree(goldfish_pipe_dev.pipes);
free_page((unsigned long)goldfish_pipe_dev.buffers);
--
2.19.0.605.g01d371f741-goog


2018-10-02 23:11:36

by Roman Kiryanov

[permalink] [raw]
Subject: [PATCH v2 03/15] platform: goldfish: pipe: Move the file-scope goldfish_pipe_dev variable into the driver state

From: Roman Kiryanov <[email protected]>

This is the last patch in the series of patches to move file-scope
variables into the driver state. This change will help to introduce
another version of the pipe driver (with different state) for the
older host interface or having several instances of this device.

Signed-off-by: Roman Kiryanov <[email protected]>
---
Changes in v2:
- Updated the commit message.

drivers/platform/goldfish/goldfish_pipe.c | 66 +++++++++++++----------
1 file changed, 37 insertions(+), 29 deletions(-)

diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c
index 8ca709b45e1f..4013832f38fb 100644
--- a/drivers/platform/goldfish/goldfish_pipe.c
+++ b/drivers/platform/goldfish/goldfish_pipe.c
@@ -169,6 +169,9 @@ struct goldfish_pipe {
* waiting to be awoken.
*/
struct goldfish_pipe_dev {
+ /* A magic number to check if this is an instance of this struct */
+ void *magic;
+
/*
* Global device spinlock. Protects the following members:
* - pipes, pipes_capacity
@@ -215,8 +218,6 @@ struct goldfish_pipe_dev {
struct miscdevice miscdev;
};

-static struct goldfish_pipe_dev goldfish_pipe_dev;
-
static int goldfish_pipe_cmd_locked(struct goldfish_pipe *pipe,
enum PipeCmdCode cmd)
{
@@ -611,6 +612,9 @@ static void goldfish_interrupt_task(unsigned long dev_addr)
}
}

+static void goldfish_pipe_device_deinit(struct platform_device *pdev,
+ struct goldfish_pipe_dev *dev);
+
/*
* The general idea of the interrupt handling:
*
@@ -631,7 +635,7 @@ static irqreturn_t goldfish_pipe_interrupt(int irq, void *dev_id)
unsigned long flags;
struct goldfish_pipe_dev *dev = dev_id;

- if (dev != &goldfish_pipe_dev)
+ if (dev->magic != &goldfish_pipe_device_deinit)
return IRQ_NONE;

/* Request the signalled pipes from the device */
@@ -683,6 +687,14 @@ static int get_free_pipe_id_locked(struct goldfish_pipe_dev *dev)
return id;
}

+/* A helper function to get the instance of goldfish_pipe_dev from file */
+static struct goldfish_pipe_dev *to_goldfish_pipe_dev(struct file *file)
+{
+ struct miscdevice *miscdev = file->private_data;
+
+ return container_of(miscdev, struct goldfish_pipe_dev, miscdev);
+}
+
/**
* goldfish_pipe_open - open a channel to the AVD
* @inode: inode of device
@@ -696,7 +708,7 @@ static int get_free_pipe_id_locked(struct goldfish_pipe_dev *dev)
*/
static int goldfish_pipe_open(struct inode *inode, struct file *file)
{
- struct goldfish_pipe_dev *dev = &goldfish_pipe_dev;
+ struct goldfish_pipe_dev *dev = to_goldfish_pipe_dev(file);
unsigned long flags;
int id;
int status;
@@ -804,9 +816,9 @@ static void write_pa_addr(void *addr, void __iomem *portl, void __iomem *porth)
writel(lower_32_bits(paddr), portl);
}

-static int goldfish_pipe_device_init(struct platform_device *pdev)
+static int goldfish_pipe_device_init(struct platform_device *pdev,
+ struct goldfish_pipe_dev *dev)
{
- struct goldfish_pipe_dev *dev = &goldfish_pipe_dev;
int err;

tasklet_init(&dev->irq_tasklet, &goldfish_interrupt_task,
@@ -861,26 +873,29 @@ static int goldfish_pipe_device_init(struct platform_device *pdev)
dev->base + PIPE_REG_OPEN_BUFFER,
dev->base + PIPE_REG_OPEN_BUFFER_HIGH);

+ platform_set_drvdata(pdev, dev);
return 0;
}

-static void goldfish_pipe_device_deinit(struct platform_device *pdev)
+static void goldfish_pipe_device_deinit(struct platform_device *pdev,
+ struct goldfish_pipe_dev *dev)
{
- misc_deregister(&goldfish_pipe_dev.miscdev);
- tasklet_kill(&goldfish_pipe_dev.irq_tasklet);
- kfree(goldfish_pipe_dev.pipes);
- free_page((unsigned long)goldfish_pipe_dev.buffers);
+ misc_deregister(&dev->miscdev);
+ tasklet_kill(&dev->irq_tasklet);
+ kfree(dev->pipes);
+ free_page((unsigned long)dev->buffers);
}

static int goldfish_pipe_probe(struct platform_device *pdev)
{
- int err;
struct resource *r;
- struct goldfish_pipe_dev *dev = &goldfish_pipe_dev;
+ struct goldfish_pipe_dev *dev;

- /* not thread safe, but this should not happen */
- WARN_ON(dev->base);
+ dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
+ if (!dev)
+ return -ENOMEM;

+ dev->magic = &goldfish_pipe_device_deinit;
spin_lock_init(&dev->lock);

r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -895,10 +910,9 @@ static int goldfish_pipe_probe(struct platform_device *pdev)
}

r = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
- if (!r) {
- err = -EINVAL;
- goto error;
- }
+ if (!r)
+ return -EINVAL;
+
dev->irq = r->start;

/*
@@ -913,20 +927,14 @@ static int goldfish_pipe_probe(struct platform_device *pdev)
if (WARN_ON(dev->version < PIPE_CURRENT_DEVICE_VERSION))
return -EINVAL;

- err = goldfish_pipe_device_init(pdev);
- if (!err)
- return 0;
-
-error:
- dev->base = NULL;
- return err;
+ return goldfish_pipe_device_init(pdev, dev);
}

static int goldfish_pipe_remove(struct platform_device *pdev)
{
- struct goldfish_pipe_dev *dev = &goldfish_pipe_dev;
- goldfish_pipe_device_deinit(pdev);
- dev->base = NULL;
+ struct goldfish_pipe_dev *dev = platform_get_drvdata(pdev);
+
+ goldfish_pipe_device_deinit(pdev, dev);
return 0;
}

--
2.19.0.605.g01d371f741-goog


2018-10-03 11:06:01

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2 01/15] platform: goldfish: pipe: Move the file-scope goldfish_interrupt_tasklet variable into the driver state

On Tue, Oct 02, 2018 at 04:10:22PM -0700, [email protected] wrote:
> From: Roman Kiryanov <[email protected]>
>
> This is a series of patches to move mutable file-scope variables
> into the driver state. This change will help to introduce another
> version of the pipe driver (with different state) for the older
> host interface or having several instances of this device.
>
> Signed-off-by: Roman Kiryanov <[email protected]>
> ---
> Changes in v2:
> - Updated the commit message.

I only received 3 emails in this series. Always resend them all.

thanks,

greg k-h

2018-10-03 16:27:33

by Roman Kiryanov

[permalink] [raw]
Subject: Re: [PATCH v2 01/15] platform: goldfish: pipe: Move the file-scope goldfish_interrupt_tasklet variable into the driver state

> I only received 3 emails in this series. Always resend them all.

Hi. It was not obvious to me to resend all patches.

Do I mark all other patches "v2" as well? What do I put there in the
"Changelog"?

2018-10-03 16:37:31

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2 01/15] platform: goldfish: pipe: Move the file-scope goldfish_interrupt_tasklet variable into the driver state

On Wed, Oct 03, 2018 at 09:26:12AM -0700, Roman Kiryanov wrote:
> > I only received 3 emails in this series. Always resend them all.
>
> Hi. It was not obvious to me to resend all patches.

If I find a problem in the first patch of a series, I just drop the
whole thing.

> Do I mark all other patches "v2" as well?

Yes.

> What do I put there in the "Changelog"?

"no change"

This should be all documented somewhere in our in-kernel
documentation...

thanks,

greg k-h