platform: goldfish: pipe: Remove the goldfish_interrupt_tasklet global variable
PATCH 1/15 platform: goldfish: pipe: Remove the goldfish_interrupt_tasklet global variable
From: Roman Kiryanov <[email protected]>
This is a series of patches to remove mutable global variables to introduce another version of the pipe driver for the older host interface. I don't want to have two driver states where only one is used.
Signed-off-by Roman Kiryanov <[email protected]>
--- drivers/platform/goldfish/goldfish_pipe.c | 24 +++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-)
drivers/platform/goldfish/goldfish_pipe.c +16 -8
@@ -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); }
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH 2/15 platform: goldfish: pipe: Remove the goldfish_pipe_miscdev global variable
From: Roman Kiryanov <[email protected]>
This is a series of patches to remove mutable global variables to introduce another version of the pipe driver for the older host interface. I don't want to have two driver states where only one is used.
Signed-off-by Roman Kiryanov <[email protected]>
--- drivers/platform/goldfish/goldfish_pipe.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)
drivers/platform/goldfish/goldfish_pipe.c +13 -7
@@ -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);
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH 3/15 platform: goldfish: pipe: Remove the goldfish_pipe_dev global variable
In reply to [email protected]From: Roman Kiryanov <[email protected]>
This is the last patch in the series of patches to remove mutable global variables to introduce another version of the pipe driver for the older host interface. I don't want to have two driver states where only one is used.
Signed-off-by Roman Kiryanov <[email protected]>
--- drivers/platform/goldfish/goldfish_pipe.c | 66 +++++++++++++---------- 1 file changed, 37 insertions(+), 29 deletions(-)
drivers/platform/goldfish/goldfish_pipe.c +37 -29
@@ -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 ; }
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH 4/15 platform: goldfish: pipe: Call misc_deregister if init fails
In reply to [email protected]From: Roman Kiryanov <[email protected]>
Undo effects of misc_register if driver's init fails after misc_register.
Signed-off-by Roman Kiryanov <[email protected]>
--- drivers/platform/goldfish/goldfish_pipe.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/platform/goldfish/goldfish_pipe.c +4 -1
@@ -844,8 +844,10 @@ static int goldfish_pipe_device_init(struct platform_device *pdev, dev->pipes_capacity =INITIAL_PIPES_CAPACITY ; dev->pipes =kcalloc (dev->pipes_capacity,sizeof (*dev->pipes),GFP_KERNEL ); -if (!dev->pipes) +if (!dev->pipes) { +misc_deregister (&dev->miscdev);return -ENOMEM ; + }/* * We're going to pass two buffers, open_command_params and
@@ -858,6 +860,7 @@ static int goldfish_pipe_device_init(struct platform_device *pdev,__get_free_page (GFP_KERNEL );if (!dev->buffers) {kfree (dev->pipes); +misc_deregister (&dev->miscdev);return -ENOMEM ; }
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH 5/15 platform: goldfish: pipe: Remove redundant casting
In reply to [email protected]From: Roman Kiryanov <[email protected]>
This casting is not required.
Signed-off-by Roman Kiryanov <[email protected]>
--- drivers/platform/goldfish/goldfish_pipe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/platform/goldfish/goldfish_pipe.c +1 -1
@@ -925,7 +925,7 @@ static int goldfish_pipe_probe(struct platform_device *pdev)* reading device version back: this allows the host implementation to * detect the old driver (if there was no version write before read). */ -writel ((u32 )PIPE_DRIVER_VERSION , dev->base +PIPE_REG_VERSION ); +writel (PIPE_DRIVER_VERSION , dev->base +PIPE_REG_VERSION ); dev->version =readl (dev->base +PIPE_REG_VERSION );if (WARN_ON (dev->version <PIPE_CURRENT_DEVICE_VERSION ))return -EINVAL ;
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH 6/15 platform: goldfish: pipe: Move memory allocation from probe to init
In reply to [email protected]From: Roman Kiryanov <[email protected]>
There will be two separate init functions for v1 and v2 (different driver versions) and they will allocate different state.
Signed-off-by Roman Kiryanov <[email protected]>
--- drivers/platform/goldfish/goldfish_pipe.c | 42 +++++++++++++---------- 1 file changed, 23 insertions(+), 19 deletions(-)
drivers/platform/goldfish/goldfish_pipe.c +23 -19
@@ -208,8 +208,6 @@ struct goldfish_pipe_dev {struct device *pdev_dev;/* Some device-specific data */ -int irq; -int version;unsigned char __iomem *base;/* an irq tasklet to run goldfish_interrupt_task */
@@ -817,14 +815,23 @@ 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) +char __iomem *base, +int irq) { +struct goldfish_pipe_dev *dev;int err; + dev =devm_kzalloc (&pdev->dev,sizeof (*dev),GFP_KERNEL ); +if (!dev) +return -ENOMEM ; + + dev->magic = &goldfish_pipe_device_deinit; +spin_lock_init (&dev->lock); +tasklet_init (&dev->irq_tasklet, &goldfish_interrupt_task, (unsigned long )dev); - err =devm_request_irq (&pdev->dev, dev->irq, + err =devm_request_irq (&pdev->dev, irq, goldfish_pipe_interrupt,IRQF_SHARED ,"goldfish_pipe" , dev);if (err) {
@@ -839,6 +846,7 @@ static int goldfish_pipe_device_init(struct platform_device *pdev,return err; } + dev->base = base; dev->pdev_dev = &pdev->dev; dev->first_signalled_pipe =NULL ; dev->pipes_capacity =INITIAL_PIPES_CAPACITY ;
@@ -892,22 +900,18 @@ static void goldfish_pipe_device_deinit(struct platform_device *pdev,static int goldfish_pipe_probe (struct platform_device *pdev) {struct resource *r; -struct goldfish_pipe_dev *dev; - - dev =devm_kzalloc (&pdev->dev,sizeof (*dev),GFP_KERNEL ); -if (!dev) -return -ENOMEM ; - - dev->magic = &goldfish_pipe_device_deinit; -spin_lock_init (&dev->lock); +char __iomem *base; +int irq; +int version; r =platform_get_resource (pdev,IORESOURCE_MEM ,0 );if (!r ||resource_size (r) <PAGE_SIZE ) {dev_err (&pdev->dev,"can't allocate i/o page\n" );return -EINVAL ; } - dev->base =devm_ioremap (&pdev->dev, r->start,PAGE_SIZE ); -if (!dev->base) { + + base =devm_ioremap (&pdev->dev, r->start,PAGE_SIZE ); +if (!base) {dev_err (&pdev->dev,"ioremap failed\n" );return -EINVAL ; }
@@ -916,7 +920,7 @@ static int goldfish_pipe_probe(struct platform_device *pdev)if (!r)return -EINVAL ; - dev->irq = r->start; + irq = r->start;/* * Exchange the versions with the host device
@@ -925,12 +929,12 @@ static int goldfish_pipe_probe(struct platform_device *pdev)* reading device version back: this allows the host implementation to * detect the old driver (if there was no version write before read). */ -writel (PIPE_DRIVER_VERSION , dev->base +PIPE_REG_VERSION ); - dev->version =readl (dev->base +PIPE_REG_VERSION ); -if (WARN_ON (dev->version <PIPE_CURRENT_DEVICE_VERSION )) +writel (PIPE_DRIVER_VERSION , base +PIPE_REG_VERSION ); + version =readl (base +PIPE_REG_VERSION ); +if (WARN_ON (version <PIPE_CURRENT_DEVICE_VERSION ))return -EINVAL ; -return goldfish_pipe_device_init (pdev, dev); +return goldfish_pipe_device_init (pdev, base, irq); }static int goldfish_pipe_remove (struct platform_device *pdev)
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH 7/15 platform: goldfish: pipe: Return status from "deinit" since "remove" does not do much
In reply to [email protected]From: Roman Kiryanov <[email protected]>
This way deinit will have a chance to report an error.
Signed-off-by Roman Kiryanov <[email protected]>
--- drivers/platform/goldfish/goldfish_pipe.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
drivers/platform/goldfish/goldfish_pipe.c +5 -4
@@ -888,13 +888,15 @@ static int goldfish_pipe_device_init(struct platform_device *pdev,return 0 ; } -static void goldfish_pipe_device_deinit (struct platform_device *pdev, -struct goldfish_pipe_dev *dev) +static int goldfish_pipe_device_deinit (struct platform_device *pdev, +struct goldfish_pipe_dev *dev) {misc_deregister (&dev->miscdev);tasklet_kill (&dev->irq_tasklet);kfree (dev->pipes);free_page ((unsigned long )dev->buffers); + +return 0 ; }static int goldfish_pipe_probe (struct platform_device *pdev)
@@ -941,8 +943,7 @@ static int goldfish_pipe_remove(struct platform_device *pdev) {struct goldfish_pipe_dev *dev =platform_get_drvdata (pdev); -goldfish_pipe_device_deinit (pdev, dev); -return 0 ; +return goldfish_pipe_device_deinit (pdev, dev); }static const struct acpi_device_id goldfish_pipe_acpi_match[] = {
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH 8/15 platform: goldfish: pipe: Add a blank line to separate varibles and code
In reply to [email protected]From: Roman Kiryanov <[email protected]>
checkpacth: Missing a blank line after declarations
Signed-off-by Roman Kiryanov <[email protected]>
--- drivers/platform/goldfish/goldfish_pipe.c | 1 + 1 file changed, 1 insertion(+)
drivers/platform/goldfish/goldfish_pipe.c +1 -0
@@ -713,6 +713,7 @@ static int goldfish_pipe_open(struct inode *inode, struct file *file)/* Allocate new pipe kernel object */ struct goldfish_pipe *pipe =kzalloc (sizeof (*pipe),GFP_KERNEL ); +if (!pipe)return -ENOMEM ;
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH 9/15 platform: goldfish: pipe: Move goldfish_pipe to goldfish_pipe_v2
In reply to [email protected]From: Roman Kiryanov <[email protected]>
This is the v2 driver. v1 will be added later.
Signed-off-by Roman Kiryanov <[email protected]>
---
drivers/platform/goldfish/Makefile | 2 +-
.../platform/goldfish/{goldfish_pipe.c => goldfish_pipe_v2.c} | 0
2 files changed, 1 insertion(+), 1 deletion(-)
rename drivers/platform/goldfish/{goldfish_pipe.c => goldfish_pipe_v2.c} (100%)drivers/platform/goldfish/Makefile +1 -1
@@ -1,4 +1,4 @@ # # Makefile for Goldfish platform specific drivers # -obj-$(CONFIG_GOLDFISH_PIPE) += goldfish_pipe.o +obj-$(CONFIG_GOLDFISH_PIPE) += goldfish_pipe_v2.o
drivers/platform/goldfish/goldfish_pipe_v2.c renamed from drivers/platform/goldfish/goldfish_pipe.c +0 -0
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH 10/15 platform: goldfish: pipe: Remove the license boilerplate
In reply to [email protected]From: Roman Kiryanov <[email protected]>
Not required with the SPDX-License-Identifier header.
Signed-off-by Roman Kiryanov <[email protected]>
--- drivers/platform/goldfish/goldfish_pipe_v2.c | 17 ----------------- 1 file changed, 17 deletions(-)
drivers/platform/goldfish/goldfish_pipe_v2.c +0 -17
@@ -1,21 +1,4 @@// SPDX-License-Identifier: GPL-2.0 -/* -* Copyright (C) 2012 Intel, Inc. -* Copyright (C) 2013 Intel, Inc. -* Copyright (C) 2014 Linaro Limited -* Copyright (C) 2011-2016 Google, Inc. -* -* This software is licensed under the terms of the GNU General Public -* License version 2, as published by the Free Software Foundation, and -* may be copied, distributed, and modified under those terms. -* -* This program is distributed in the hope that it will be useful, -* but WITHOUT ANY WARRANTY; without even the implied warranty of -* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -* GNU General Public License for more details. -* -*/ -/* This source file contains the implementation of a special device driver * that intends to provide a *very* fast communication channel between the * guest system and the QEMU emulator.
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH 11/15 platform: goldfish: pipe: Split the driver to v2 specific and the rest
In reply to [email protected]From: Roman Kiryanov <[email protected]>
Move probe/remove and other driver stuff to a separate file to plug v1 there later.
Signed-off-by Roman Kiryanov <[email protected]>
--- drivers/platform/goldfish/Makefile | 3 +- drivers/platform/goldfish/goldfish_pipe.c | 124 +++++++++++++++++++ drivers/platform/goldfish/goldfish_pipe.h | 10 ++ drivers/platform/goldfish/goldfish_pipe_v2.c | 112 +++-------------- drivers/platform/goldfish/goldfish_pipe_v2.h | 10 ++ 5 files changed, 161 insertions(+), 98 deletions(-) create mode 100644 drivers/platform/goldfish/goldfish_pipe.c create mode 100644 drivers/platform/goldfish/goldfish_pipe.h create mode 100644 drivers/platform/goldfish/goldfish_pipe_v2.h
drivers/platform/goldfish/Makefile +2 -1
@@ -1,4 +1,5 @@ # # Makefile for Goldfish platform specific drivers # -obj-$(CONFIG_GOLDFISH_PIPE) += goldfish_pipe_v2.o +obj-$(CONFIG_GOLDFISH_PIPE) += goldfish_pipe_all.o +goldfish_pipe_all-objs := goldfish_pipe.o goldfish_pipe_v2.o
drivers/platform/goldfish/goldfish_pipe.c new file +124 -0
124 lines not on this page · raw message
drivers/platform/goldfish/goldfish_pipe.h new file +10 -0
@@ -0,0 +1,10 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef GOLDFISH_PIPE_H +#define GOLDFISH_PIPE_H + +struct goldfish_pipe_dev_base { +/* the destructor, the pointer is set in init */ +int (*deinit)(void *pipe_dev,struct platform_device *pdev); +}; + +#endif /* GOLDFISH_PIPE_H */
drivers/platform/goldfish/goldfish_pipe_v2.c +15 -97
172 lines not on this page · raw message
drivers/platform/goldfish/goldfish_pipe_v2.h new file +10 -0
@@ -0,0 +1,10 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef GOLDFISH_PIPE_V2_H +#define GOLDFISH_PIPE_V2_H + +/* The entry point to the pipe v2 driver */ +int goldfish_pipe_device_init (struct platform_device *pdev, +char __iomem *base, +int irq); + +#endif /* #define GOLDFISH_PIPE_V2_H */
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH 12/15 platform: goldfish: pipe: Rename the init function (add "v2")
In reply to [email protected]From: Roman Kiryanov <[email protected]>
This is the v2 driver. v1 will be added later.
Signed-off-by Roman Kiryanov <[email protected]>
--- drivers/platform/goldfish/goldfish_pipe.c | 2 +- drivers/platform/goldfish/goldfish_pipe_v2.c | 6 +++--- drivers/platform/goldfish/goldfish_pipe_v2.h | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-)
drivers/platform/goldfish/goldfish_pipe.c +1 -1
@@ -87,7 +87,7 @@ static int goldfish_pipe_probe(struct platform_device *pdev)if (WARN_ON (version <PIPE_CURRENT_DEVICE_VERSION ))return -EINVAL ; -return goldfish_pipe_device_init (pdev, base, irq); +return goldfish_pipe_device_v2_init (pdev, base, irq); }static int goldfish_pipe_remove (struct platform_device *pdev)
drivers/platform/goldfish/goldfish_pipe_v2.c +3 -3
@@ -787,9 +787,9 @@ static void write_pa_addr(void *addr, void __iomem *portl, void __iomem *porth)writel (lower_32_bits (paddr), portl); } -int goldfish_pipe_device_init (struct platform_device *pdev, -char __iomem *base, -int irq) +int goldfish_pipe_device_v2_init (struct platform_device *pdev, +char __iomem *base, +int irq) {struct goldfish_pipe_dev *dev;int err;
drivers/platform/goldfish/goldfish_pipe_v2.h +3 -3
@@ -3,8 +3,8 @@#define GOLDFISH_PIPE_V2_H /* The entry point to the pipe v2 driver */ -int goldfish_pipe_device_init (struct platform_device *pdev, -char __iomem *base, -int irq); +int goldfish_pipe_device_v2_init (struct platform_device *pdev, +char __iomem *base, +int irq);#endif /* #define GOLDFISH_PIPE_V2_H */
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH 13/15 platform: goldfish: pipe: Add a dedicated constant for the device name
In reply to [email protected]From: Roman Kiryanov <[email protected]>
Create a constant to refer to the device name instead if several copies of a string.
Signed-off-by Roman Kiryanov <[email protected]>
--- drivers/platform/goldfish/goldfish_pipe.h | 2 ++ drivers/platform/goldfish/goldfish_pipe_v2.c | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-)
drivers/platform/goldfish/goldfish_pipe.h +2 -0
@@ -2,6 +2,8 @@ #ifndef GOLDFISH_PIPE_H #define GOLDFISH_PIPE_H +#define DEVICE_NAME "goldfish_pipe" + struct goldfish_pipe_dev_base { /* the destructor, the pointer is set in init */ int (*deinit)(void *pipe_dev, struct platform_device *pdev);
drivers/platform/goldfish/goldfish_pipe_v2.c +2 -2
@@ -775,7 +775,7 @@ static void init_miscdevice(struct miscdevice *miscdev)memset (miscdev,0 ,sizeof (*miscdev)); miscdev->minor =MISC_DYNAMIC_MINOR ; - miscdev->name ="goldfish_pipe" ; + miscdev->name =DEVICE_NAME ; miscdev->fops = &goldfish_pipe_fops; }
@@ -806,7 +806,7 @@ int goldfish_pipe_device_v2_init(struct platform_device *pdev, err =devm_request_irq (&pdev->dev, irq, goldfish_pipe_interrupt, -IRQF_SHARED ,"goldfish_pipe" , dev); +IRQF_SHARED ,DEVICE_NAME , dev);if (err) {dev_err (&pdev->dev,"unable to allocate IRQ for v2\n" );return err;
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH 14/15 platform: goldfish: pipe: Rename PIPE_REG to PIPE_V2_REG
In reply to [email protected]From: Roman Kiryanov <[email protected]>
PIPE_V1_REG will be introduced later for v1 support.
Signed-off-by Roman Kiryanov <[email protected]>
--- drivers/platform/goldfish/goldfish_pipe.c | 4 ++-- drivers/platform/goldfish/goldfish_pipe_qemu.h | 18 +++++++++--------- drivers/platform/goldfish/goldfish_pipe_v2.c | 16 ++++++++-------- 3 files changed, 19 insertions(+), 19 deletions(-)
drivers/platform/goldfish/goldfish_pipe.c +2 -2
@@ -81,8 +81,8 @@ static int goldfish_pipe_probe(struct platform_device *pdev)* reading device version back: this allows the host implementation to * detect the old driver (if there was no version write before read). */ -writel (PIPE_DRIVER_VERSION , base +PIPE_REG_VERSION ); - version =readl (base +PIPE_REG_VERSION ); +writel (PIPE_DRIVER_VERSION , base +PIPE_V2_REG_VERSION ); + version =readl (base +PIPE_V2_REG_VERSION );if (WARN_ON (version <PIPE_CURRENT_DEVICE_VERSION ))return -EINVAL ;
drivers/platform/goldfish/goldfish_pipe_qemu.h +9 -9
@@ -62,19 +62,19 @@ enum PipeFlagsBits { BIT_WAKE_ON_READ = 2, /* want to be woken on reads */ }; -enum PipeRegs { -PIPE_REG_CMD =0 , +enum PipeV2Regs { +PIPE_V2_REG_CMD =0 , -PIPE_REG_SIGNAL_BUFFER_HIGH =4 , -PIPE_REG_SIGNAL_BUFFER =8 , -PIPE_REG_SIGNAL_BUFFER_COUNT =12 , +PIPE_V2_REG_SIGNAL_BUFFER_HIGH =4 , +PIPE_V2_REG_SIGNAL_BUFFER =8 , +PIPE_V2_REG_SIGNAL_BUFFER_COUNT =12 , -PIPE_REG_OPEN_BUFFER_HIGH =20 , -PIPE_REG_OPEN_BUFFER =24 , +PIPE_V2_REG_OPEN_BUFFER_HIGH =20 , +PIPE_V2_REG_OPEN_BUFFER =24 , -PIPE_REG_VERSION =36 , +PIPE_V2_REG_VERSION =36 , -PIPE_REG_GET_SIGNALLED =48 , +PIPE_V2_REG_GET_SIGNALLED =48 , };enum PipeCmdCode {
drivers/platform/goldfish/goldfish_pipe_v2.c +8 -8
@@ -197,7 +197,7 @@ static int goldfish_pipe_cmd_locked(struct goldfish_pipe *pipe, pipe->command_buffer->cmd = cmd;/* failure by default */ pipe->command_buffer->status =PIPE_ERROR_INVAL ; -writel (pipe->id, pipe->dev->base +PIPE_REG_CMD ); +writel (pipe->id, pipe->dev->base +PIPE_V2_REG_CMD );return pipe->command_buffer->status; }
@@ -214,7 +214,7 @@ static int goldfish_pipe_cmd(struct goldfish_pipe *pipe, enum PipeCmdCode cmd)/* * This function converts an error code returned by the emulator through -* the PIPE_REG_STATUS i/o register into a valid negative errno value. +* the PIPE_V2_REG_STATUS i/o register into a valid negative errno value. */ static int goldfish_pipe_error_convert (int status) {
@@ -611,7 +611,7 @@ static irqreturn_t goldfish_pipe_interrupt(int irq, void *dev_id)/* Request the signalled pipes from the device */ spin_lock_irqsave (&dev->lock, flags); - count =readl (dev->base +PIPE_REG_GET_SIGNALLED ); + count =readl (dev->base +PIPE_V2_REG_GET_SIGNALLED );if (count ==0 ) {spin_unlock_irqrestore (&dev->lock, flags);return IRQ_NONE ;
@@ -847,15 +847,15 @@ int goldfish_pipe_device_v2_init(struct platform_device *pdev,/* Send the buffer addresses to the host */ write_pa_addr (&dev->buffers->signalled_pipe_buffers, - dev->base +PIPE_REG_SIGNAL_BUFFER , - dev->base +PIPE_REG_SIGNAL_BUFFER_HIGH ); + dev->base +PIPE_V2_REG_SIGNAL_BUFFER , + dev->base +PIPE_V2_REG_SIGNAL_BUFFER_HIGH );writel (MAX_SIGNALLED_PIPES , - dev->base +PIPE_REG_SIGNAL_BUFFER_COUNT ); + dev->base +PIPE_V2_REG_SIGNAL_BUFFER_COUNT );write_pa_addr (&dev->buffers->open_command_params, - dev->base +PIPE_REG_OPEN_BUFFER , - dev->base +PIPE_REG_OPEN_BUFFER_HIGH ); + dev->base +PIPE_V2_REG_OPEN_BUFFER , + dev->base +PIPE_V2_REG_OPEN_BUFFER_HIGH );platform_set_drvdata (pdev, dev);return 0 ;
Signature · 1 line
2.19.0.605.g01d371f741-goog
PATCH 15/15 platform: goldfish: pipe: Add the goldfish_pipe_v1 driver
In reply to [email protected]From: Roman Kiryanov <[email protected]>
This is the v1 goldfish pipe driver.
Signed-off-by Roman Kiryanov <[email protected]>
--- drivers/platform/goldfish/Makefile | 2 +- drivers/platform/goldfish/goldfish_pipe.c | 9 +- .../platform/goldfish/goldfish_pipe_qemu.h | 27 + drivers/platform/goldfish/goldfish_pipe_v1.c | 632 ++++++++++++++++++ drivers/platform/goldfish/goldfish_pipe_v1.h | 24 + 5 files changed, 689 insertions(+), 5 deletions(-) create mode 100644 drivers/platform/goldfish/goldfish_pipe_v1.c create mode 100644 drivers/platform/goldfish/goldfish_pipe_v1.h
drivers/platform/goldfish/Makefile +1 -1
@@ -2,4 +2,4 @@ # Makefile for Goldfish platform specific drivers # obj-$(CONFIG_GOLDFISH_PIPE) += goldfish_pipe_all.o -goldfish_pipe_all-objs := goldfish_pipe.o goldfish_pipe_v2.o +goldfish_pipe_all-objs := goldfish_pipe.o goldfish_pipe_v1.o goldfish_pipe_v2.o
drivers/platform/goldfish/goldfish_pipe.c +5 -4
@@ -39,6 +39,7 @@#include <linux/acpi.h> #include "goldfish_pipe_qemu.h" #include "goldfish_pipe.h" +#include "goldfish_pipe_v1.h" #include "goldfish_pipe_v2.h" /*
@@ -84,10 +85,10 @@ static int goldfish_pipe_probe(struct platform_device *pdev)writel (PIPE_DRIVER_VERSION , base +PIPE_V2_REG_VERSION ); version =readl (base +PIPE_V2_REG_VERSION ); -if (WARN_ON (version <PIPE_CURRENT_DEVICE_VERSION )) -return -EINVAL ; - -return goldfish_pipe_device_v2_init (pdev, base, irq); +if (version <PIPE_CURRENT_DEVICE_VERSION ) +return goldfish_pipe_device_v1_init (pdev, base, irq); +else +return goldfish_pipe_device_v2_init (pdev, base, irq); }static int goldfish_pipe_remove (struct platform_device *pdev)
drivers/platform/goldfish/goldfish_pipe_qemu.h +27 -0
@@ -62,6 +62,33 @@ enum PipeFlagsBits { BIT_WAKE_ON_READ = 2, /* want to be woken on reads */ }; +enum PipeV1Regs { +/* write: value = command */ +PIPE_V1_REG_COMMAND =0x00 , +/* read */ +PIPE_V1_REG_STATUS =0x04 , +/* read/write: channel id */ +PIPE_V1_REG_CHANNEL =0x08 , +/* read/write: channel id */ +PIPE_V1_REG_CHANNEL_HIGH =0x30 , +/* read/write: buffer size */ +PIPE_V1_REG_SIZE =0x0C , +/* write: physical address */ +PIPE_V1_REG_ADDRESS =0x10 , +/* write: physical address */ +PIPE_V1_REG_ADDRESS_HIGH =0x34 , +/* read: wake flags */ +PIPE_V1_REG_WAKES =0x14 , +/* read/write: batch data address */ +PIPE_V1_REG_PARAMS_ADDR_LOW =0x18 , +/* read/write: batch data address */ +PIPE_V1_REG_PARAMS_ADDR_HIGH =0x1C , +/* write: batch access */ +PIPE_V1_REG_ACCESS_PARAMS =0x20 , +/* read: device version */ +PIPE_V1_REG_VERSION =0x24 , +}; +enum PipeV2Regs {PIPE_V2_REG_CMD =0 ,
drivers/platform/goldfish/goldfish_pipe_v1.c new file +632 -0
632 lines not on this page · raw message
drivers/platform/goldfish/goldfish_pipe_v1.h new file +24 -0
@@ -0,0 +1,24 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* +* Copyright (C) 2018 Google, Inc. +* +* This software is licensed under the terms of the GNU General Public +* License version 2, as published by the Free Software Foundation, and +* may be copied, distributed, and modified under those terms. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +*/ + +#ifndef GOLDFISH_PIPE_V1_H +#define GOLDFISH_PIPE_V1_H + +/* The entry point to the pipe v1 driver */ +int goldfish_pipe_device_v1_init (struct platform_device *pdev, +void __iomem *base, +int irq); + +#endif /* #define GOLDFISH_PIPE_V1_H */
Signature · 1 line
2.19.0.605.g01d371f741-goog
Quoted 6 lines · On Tue, Oct 02, 2018 at 03:18:49PM -0700, [email protected] wrote:
From: Roman Kiryanov <[email protected]>
This is a series of patches to remove mutable global variables to introduce another version of the pipe driver for the older host interface. I don't want to have two driver states where only one is used.
I don't see a "global" variable here, what are you referring to.
This one:
Quoted 1 line
-static
DECLARE_TASKLET(goldfish_interrupt_tasklet, goldfish_interrupt_task, 0);
?
That looks static to this file to me.
what am I missing here?
greg k-h
Quoted 7 lines
I don't see a "global" variable here, what are you referring to.
This one:
Quoted 1 line
-static
DECLARE_TASKLET(goldfish_interrupt_tasklet, goldfish_interrupt_task, 0);?
Yes.
Quoted 2 lines
That looks static to this file to me.
what am I missing here?
It has a global lifetime but it is visible only in this file. How do I say better, just drop the word "global"?
Quoted 8 lines · Roman Kiryanov
Quoted 7 lines
I don't see a "global" variable here, what are you referring to.
This one:
Quoted 1 line
-static
DECLARE_TASKLET(goldfish_interrupt_tasklet, goldfish_interrupt_task, 0);?
Quoted 7 lines
Yes.
Quoted 2 lines
That looks static to this file to me.
what am I missing here?It has a global lifetime but it is visible only in this file. How do I say better, just drop the word "global"?
Yes. It's not a global variable at all. It's file-scope only.
What you are doing is moving the variable to be attached to the device itself, allowing you to have multiple devices handled by the same code, which is great. But please document it as such.
Same goes for the other patches in this series.
thanks,
greg k-h