Re: [PATCH RESEND v10 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Mon, Aug 17, 2020 at 05:53:09PM -0400, Jim Quinlan wrote:
> The new field 'dma_range_map' in struct device is used to facilitate the
> use of single or multiple offsets between mapping regions of cpu addrs and
> dma addrs. It subsumes the role of "dev->dma_pfn_offset" which was only
> capable of holding a single uniform offset and had no region bounds
> checking.
>
> The function of_dma_get_range() has been modified so that it takes a single
> argument -- the device node -- and returns a map, NULL, or an error code.
> The map is an array that holds the information regarding the DMA regions.
> Each range entry contains the address offset, the cpu_start address, the
> dma_start address, and the size of the region.
>
> of_dma_configure() is the typical manner to set range offsets but there are
> a number of ad hoc assignments to "dev->dma_pfn_offset" in the kernel
> driver code. These cases now invoke the function
> dma_attach_offset_range(dev, cpu_addr, dma_addr, size).
...
> + if (dev) {
> + phys_addr_t paddr = PFN_PHYS(pfn);
> +
> + pfn -= (dma_offset_from_phys_addr(dev, paddr) >> PAGE_SHIFT);
PFN_DOWN() ?
> + }
...
> + pfn += (dma_offset_from_dma_addr(dev, addr) >> PAGE_SHIFT);
Ditto.
...
> +static inline u64 dma_offset_from_dma_addr(struct device *dev, dma_addr_t
> dma_addr)
> +{
> + const struct bus_dma_region *m = dev->dma_range_map;
> +
> + if (!m)
> + return 0;
> + for (; m->size; m++)
> + if (dma_addr >= m->dma_start && dma_addr - m->dma_start <
> m->size)
> + return m->offset;
> + return 0;
> +}
> +
> +static inline u64 dma_offset_from_phys_addr(struct device *dev, phys_addr_t
> paddr)
> +{
> + const struct bus_dma_region *m = dev->dma_range_map;
> +
> + if (!m)
> + return 0;
> + for (; m->size; m++)
> + if (paddr >= m->cpu_start && paddr - m->cpu_start < m->size)
> + return m->offset;
> + return 0;
> +}
Perhaps for these the form with one return 0 is easier to read
if (m) {
for (; m->size; m++)
if (paddr >= m->cpu_start && paddr - m->cpu_start <
m->size)
return m->offset;
}
return 0;
?
...
> + if (mem->use_dev_dma_pfn_offset) {
> + u64 base_addr = (u64)mem->pfn_base << PAGE_SHIFT;
PFN_PHYS() ?
> +
> + return base_addr - dma_offset_from_phys_addr(dev, base_addr);
> + }
...
> + * It returns -ENOMEM if out of memory, 0 otherwise.
This doesn't describe cases dev->dma_range_map != NULL and offset == 0.
> +int dma_set_offset_range(struct device *dev, phys_addr_t cpu_start,
> + dma_addr_t dma_start, u64 size)
> +{
> + struct bus_dma_region *map;
> + u64 offset = (u64)cpu_start - (u64)dma_start;
> +
> + if (!offset)
> + return 0;
> +
> + if (dev->dma_range_map) {
> + dev_err(dev, "attempt to add DMA range to existing map\n");
> + return -EINVAL;
> + }
> +
> + map = kcalloc(2, sizeof(*map), GFP_KERNEL);
> + if (!map)
> + return -ENOMEM;
> + map[0].cpu_start = cpu_start;
> + map[0].dma_start = dma_start;
> + map[0].offset = offset;
> + map[0].size = size;
> + dev->dma_range_map = map;
> +
> + return 0;
> +}
...
> +void *dma_copy_dma_range_map(const struct bus_dma_region *map)
> +{
> + int num_ranges;
> + struct bus_dma_region *new_map;
> + const struct bus_dma_region *r = map;
> +
> + for (num_ranges = 0; r->size; num_ranges++)
> + r++;
> + new_map = kcalloc(num_ranges + 1, sizeof(*map), GFP_KERNEL);
> + if (new_map)
> + memcpy(new_map, map, sizeof(*map) * num_ranges);
Looks like krealloc() on the first glance...
> +
> + return new_map;
> +}
--
With Best Regards,
Andy Shevchenko
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/4] *** SUBJECT HERE ***
*** BLURB HERE *** Tomer Samara (4): staging: android: Replace BUG_ON with WARN_ON staging: android: Add error handling to ion_page_pool_shrink staging: android: Convert BUG to WARN staging: android: Add error handling to order_to_index callers drivers/staging/android/ion/ion_page_pool.c | 14 ++ drivers/staging/android/ion/ion_system_heap.c | 16 +--- 2 files changed, 23 insertions(+), 7 deletions(-) -- 2.25.1 /tmp/0001-staging-android-Replace-BUG_ON-with-WARN_ON.patch /tmp/0002-staging-android-Add-error-handling-to-ion_page_pool_.patch /tmp/0003-staging-android-Convert-BUG-to-WARN.patch /tmp/0004-staging-android-Add-error-handling-to-order_to_index.patch ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/4] *** SUBJECT HERE ***
*** BLURB HERE *** Tomer Samara (4): staging: android: Replace BUG_ON with WARN_ON staging: android: Add error handling to ion_page_pool_shrink staging: android: Convert BUG to WARN staging: android: Add error handling to order_to_index callers drivers/staging/android/ion/ion_page_pool.c | 14 ++ drivers/staging/android/ion/ion_system_heap.c | 16 +--- 2 files changed, 23 insertions(+), 7 deletions(-) -- 2.25.1 /tmp/0001-staging-android-Replace-BUG_ON-with-WARN_ON.patch /tmp/0002-staging-android-Add-error-handling-to-ion_page_pool_.patch /tmp/0003-staging-android-Convert-BUG-to-WARN.patch /tmp/0004-staging-android-Add-error-handling-to-order_to_index.patch ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/4] *** SUBJECT HERE ***
*** BLURB HERE *** Tomer Samara (4): staging: android: Replace BUG_ON with WARN_ON staging: android: Add error handling to ion_page_pool_shrink staging: android: Convert BUG to WARN staging: android: Add error handling to order_to_index callers drivers/staging/android/ion/ion_page_pool.c | 14 ++ drivers/staging/android/ion/ion_system_heap.c | 16 +--- 2 files changed, 23 insertions(+), 7 deletions(-) -- 2.25.1 /tmp/0001-staging-android-Replace-BUG_ON-with-WARN_ON.patch /tmp/0002-staging-android-Add-error-handling-to-ion_page_pool_.patch /tmp/0003-staging-android-Convert-BUG-to-WARN.patch /tmp/0004-staging-android-Add-error-handling-to-order_to_index.patch ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] char: ipmi: convert tasklets to use new tasklet_setup() API
> >
> > Signed-off-by: Romain Perier
> > Signed-off-by: Allen Pais
>
> This looks good to me.
>
> Reviewed-by: Corey Minyard
>
> Are you planning to push this, or do you want me to take it? If you
> want me to take it, what is the urgency?
Thanks. Well, not hurry, as long as it goes into 5.9 with all other
changes.
>
> -corey
>
> > ---
> > drivers/char/ipmi/ipmi_msghandler.c | 13 ++---
> > 1 file changed, 6 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/char/ipmi/ipmi_msghandler.c
> > b/drivers/char/ipmi/ipmi_msghandler.c
> > index 737c0b6b24ea..e1814b6a1225 100644
> > --- a/drivers/char/ipmi/ipmi_msghandler.c
> > +++ b/drivers/char/ipmi/ipmi_msghandler.c
> > @@ -39,7 +39,7 @@
> >
> > static struct ipmi_recv_msg *ipmi_alloc_recv_msg(void);
> > static int ipmi_init_msghandler(void);
> > -static void smi_recv_tasklet(unsigned long);
> > +static void smi_recv_tasklet(struct tasklet_struct *t);
> > static void handle_new_recv_msgs(struct ipmi_smi *intf);
> > static void need_waiter(struct ipmi_smi *intf);
> > static int handle_one_recv_msg(struct ipmi_smi *intf,
> > @@ -3430,9 +3430,8 @@ int ipmi_add_smi(struct module *owner,
> > intf->curr_seq = 0;
> > spin_lock_init(&intf->waiting_rcv_msgs_lock);
> > INIT_LIST_HEAD(&intf->waiting_rcv_msgs);
> > - tasklet_init(&intf->recv_tasklet,
> > - smi_recv_tasklet,
> > - (unsigned long) intf);
> > + tasklet_setup(&intf->recv_tasklet,
> > + smi_recv_tasklet);
> > atomic_set(&intf->watchdog_pretimeouts_to_deliver, 0);
> > spin_lock_init(&intf->xmit_msgs_lock);
> > INIT_LIST_HEAD(&intf->xmit_msgs);
> > @@ -4467,10 +4466,10 @@ static void handle_new_recv_msgs(struct ipmi_smi
> > *intf)
> > }
> > }
> >
> > -static void smi_recv_tasklet(unsigned long val)
> > +static void smi_recv_tasklet(struct tasklet_struct *t)
> > {
> > unsigned long flags = 0; /* keep us warning-free. */
> > - struct ipmi_smi *intf = (struct ipmi_smi *) val;
> > + struct ipmi_smi *intf = from_tasklet(intf, t, recv_tasklet);
> > int run_to_completion = intf->run_to_completion;
> > struct ipmi_smi_msg *newmsg = NULL;
> >
> > @@ -4542,7 +4541,7 @@ void ipmi_smi_msg_received(struct ipmi_smi *intf,
> > spin_unlock_irqrestore(&intf->xmit_msgs_lock, flags);
> >
> > if (run_to_completion)
> > - smi_recv_tasklet((unsigned long) intf);
> > + smi_recv_tasklet(&intf->recv_tasklet);
> > else
> > tasklet_schedule(&intf->recv_tasklet);
> > }
> > --
> > 2.17.1
> >
--
- Allen
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/4] *** SUBJECT HERE ***
*** BLURB HERE *** Tomer Samara (4): staging: android: Replace BUG_ON with WARN_ON staging: android: Add error handling to ion_page_pool_shrink staging: android: Convert BUG to WARN staging: android: Add error handling to order_to_index callers drivers/staging/android/ion/ion_page_pool.c | 14 ++ drivers/staging/android/ion/ion_system_heap.c | 16 +--- 2 files changed, 23 insertions(+), 7 deletions(-) -- 2.25.1 /tmp/0001-staging-android-Replace-BUG_ON-with-WARN_ON.patch /tmp/0002-staging-android-Add-error-handling-to-ion_page_pool_.patch /tmp/0003-staging-android-Convert-BUG-to-WARN.patch /tmp/0004-staging-android-Add-error-handling-to-order_to_index.patch ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/4] *** SUBJECT HERE ***
On Tue, Aug 18, 2020 at 12:17:08PM +0300, Tomer Samara wrote: > *** BLURB HERE *** Really? And your subject line could use some work too :( ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/4] *** SUBJECT HERE ***
On Tue, Aug 18, 2020 at 11:50:35AM +0200, Greg Kroah-Hartman wrote: > On Tue, Aug 18, 2020 at 12:17:08PM +0300, Tomer Samara wrote: > > *** BLURB HERE *** > > Really? > > And your subject line could use some work too :( > sorry for that, i've made a script for sending patchset and accidently it sents mails without contorl. Fixed that ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH -next] binderfs: make symbol 'binderfs_fs_parameters' static
The sparse tool complains as follows:
drivers/android/binderfs.c:66:32: warning:
symbol 'binderfs_fs_parameters' was not declared. Should it be static?
This variable is not used outside of binderfs.c, so this commit
marks it static.
Fixes: 095cf502b31e ("binderfs: port to new mount api")
Reported-by: Hulk Robot
Signed-off-by: Wei Yongjun
---
drivers/android/binderfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
index 7b76fefde3f8..7b4f154f07e6 100644
--- a/drivers/android/binderfs.c
+++ b/drivers/android/binderfs.c
@@ -63,7 +63,7 @@ static const struct constant_table binderfs_param_stats[] = {
{}
};
-const struct fs_parameter_spec binderfs_fs_parameters[] = {
+static const struct fs_parameter_spec binderfs_fs_parameters[] = {
fsparam_u32("max", Opt_max),
fsparam_enum("stats", Opt_stats_mode, binderfs_param_stats),
{}
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH -next] binderfs: make symbol 'binderfs_fs_parameters' static
On Tue, Aug 18, 2020 at 07:22:45PM +0800, Wei Yongjun wrote:
> The sparse tool complains as follows:
>
> drivers/android/binderfs.c:66:32: warning:
> symbol 'binderfs_fs_parameters' was not declared. Should it be static?
>
> This variable is not used outside of binderfs.c, so this commit
> marks it static.
>
> Fixes: 095cf502b31e ("binderfs: port to new mount api")
> Reported-by: Hulk Robot
> Signed-off-by: Wei Yongjun
> ---
Thanks!
Acked-by: Christian Brauner
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] char: ipmi: convert tasklets to use new tasklet_setup() API
On Tue, Aug 18, 2020 at 02:46:23PM +0530, Allen wrote:
> > >
> > > Signed-off-by: Romain Perier
> > > Signed-off-by: Allen Pais
> >
> > This looks good to me.
> >
> > Reviewed-by: Corey Minyard
> >
> > Are you planning to push this, or do you want me to take it? If you
> > want me to take it, what is the urgency?
>
> Thanks. Well, not hurry, as long as it goes into 5.9 with all other
> changes.
Ok, this is queued in my for-next branch.
-corey
>
>
> >
> > -corey
> >
> > > ---
> > > drivers/char/ipmi/ipmi_msghandler.c | 13 ++---
> > > 1 file changed, 6 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/char/ipmi/ipmi_msghandler.c
> > > b/drivers/char/ipmi/ipmi_msghandler.c
> > > index 737c0b6b24ea..e1814b6a1225 100644
> > > --- a/drivers/char/ipmi/ipmi_msghandler.c
> > > +++ b/drivers/char/ipmi/ipmi_msghandler.c
> > > @@ -39,7 +39,7 @@
> > >
> > > static struct ipmi_recv_msg *ipmi_alloc_recv_msg(void);
> > > static int ipmi_init_msghandler(void);
> > > -static void smi_recv_tasklet(unsigned long);
> > > +static void smi_recv_tasklet(struct tasklet_struct *t);
> > > static void handle_new_recv_msgs(struct ipmi_smi *intf);
> > > static void need_waiter(struct ipmi_smi *intf);
> > > static int handle_one_recv_msg(struct ipmi_smi *intf,
> > > @@ -3430,9 +3430,8 @@ int ipmi_add_smi(struct module *owner,
> > > intf->curr_seq = 0;
> > > spin_lock_init(&intf->waiting_rcv_msgs_lock);
> > > INIT_LIST_HEAD(&intf->waiting_rcv_msgs);
> > > - tasklet_init(&intf->recv_tasklet,
> > > - smi_recv_tasklet,
> > > - (unsigned long) intf);
> > > + tasklet_setup(&intf->recv_tasklet,
> > > + smi_recv_tasklet);
> > > atomic_set(&intf->watchdog_pretimeouts_to_deliver, 0);
> > > spin_lock_init(&intf->xmit_msgs_lock);
> > > INIT_LIST_HEAD(&intf->xmit_msgs);
> > > @@ -4467,10 +4466,10 @@ static void handle_new_recv_msgs(struct ipmi_smi
> > > *intf)
> > > }
> > > }
> > >
> > > -static void smi_recv_tasklet(unsigned long val)
> > > +static void smi_recv_tasklet(struct tasklet_struct *t)
> > > {
> > > unsigned long flags = 0; /* keep us warning-free. */
> > > - struct ipmi_smi *intf = (struct ipmi_smi *) val;
> > > + struct ipmi_smi *intf = from_tasklet(intf, t, recv_tasklet);
> > > int run_to_completion = intf->run_to_completion;
> > > struct ipmi_smi_msg *newmsg = NULL;
> > >
> > > @@ -4542,7 +4541,7 @@ void ipmi_smi_msg_received(struct ipmi_smi *intf,
> > > spin_unlock_irqrestore(&intf->xmit_msgs_lock, flags);
> > >
> > > if (run_to_completion)
> > > - smi_recv_tasklet((unsigned long) intf);
> > > + smi_recv_tasklet(&intf->recv_tasklet);
> > > else
> > > tasklet_schedule(&intf->recv_tasklet);
> > > }
> > > --
> > > 2.17.1
> > >
>
>
>
> --
>- Allen
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: emxx_udc: Use standard BIT() macro
Currently emxx_udc.h defines bit values using local macros. Use the standard one instead. Also, combine bit values with bitwise-or rather than addition, as suggested by Coccinelle. Signed-off-by: Alex Dewar --- drivers/staging/emxx_udc/emxx_udc.h | 456 +--- 1 file changed, 211 insertions(+), 245 deletions(-) diff --git a/drivers/staging/emxx_udc/emxx_udc.h b/drivers/staging/emxx_udc/emxx_udc.h index 9c2671cb32f7..45f076e5fdb0 100644 --- a/drivers/staging/emxx_udc/emxx_udc.h +++ b/drivers/staging/emxx_udc/emxx_udc.h @@ -52,197 +52,163 @@ int vbus_irq; #define U2F_ENABLE 1 #define U2F_DISABLE0 -/*--- BIT */ -#define BIT00 0x0001 -#define BIT01 0x0002 -#define BIT02 0x0004 -#define BIT03 0x0008 -#define BIT04 0x0010 -#define BIT05 0x0020 -#define BIT06 0x0040 -#define BIT07 0x0080 -#define BIT08 0x0100 -#define BIT09 0x0200 -#define BIT10 0x0400 -#define BIT11 0x0800 -#define BIT12 0x1000 -#define BIT13 0x2000 -#define BIT14 0x4000 -#define BIT15 0x8000 -#define BIT16 0x0001 -#define BIT17 0x0002 -#define BIT18 0x0004 -#define BIT19 0x0008 -#define BIT20 0x0010 -#define BIT21 0x0020 -#define BIT22 0x0040 -#define BIT23 0x0080 -#define BIT24 0x0100 -#define BIT25 0x0200 -#define BIT26 0x0400 -#define BIT27 0x0800 -#define BIT28 0x1000 -#define BIT29 0x2000 -#define BIT30 0x4000 -#define BIT31 0x8000 - -#define TEST_FORCE_ENABLE (BIT18 + BIT16) - -#define INT_SELBIT10 -#define CONSTFSBIT09 -#define SOF_RCVBIT08 -#define RSUM_INBIT07 -#define SUSPENDBIT06 -#define CONF BIT05 -#define DEFAULTBIT04 -#define CONNECTB BIT03 -#define PUE2 BIT02 +#define TEST_FORCE_ENABLE (BIT(18) | BIT(16)) + +#define INT_SELBIT(10) +#define CONSTFSBIT(9) +#define SOF_RCVBIT(8) +#define RSUM_INBIT(7) +#define SUSPENDBIT(6) +#define CONF BIT(5) +#define DEFAULTBIT(4) +#define CONNECTB BIT(3) +#define PUE2 BIT(2) #define MAX_TEST_MODE_NUM 0x05 #define TEST_MODE_SHIFT16 /*--- (0x0004) USB Status Register */ -#define SPEED_MODE BIT06 -#define HIGH_SPEED BIT06 +#define SPEED_MODE BIT(6) +#define HIGH_SPEED BIT(6) -#define CONF BIT05 -#define DEFAULTBIT04 -#define USB_RSTBIT03 -#define SPND_OUT BIT02 -#define RSUM_OUT BIT01 +#define CONF BIT(5) +#define DEFAULTBIT(4) +#define USB_RSTBIT(3) +#define SPND_OUT BIT(2) +#define RSUM_OUT BIT(1) /*--- (0x0008) USB Address Register */ #define USB_ADDR 0x007F -#define SOF_STATUS BIT15 -#define UFRAME (BIT14 + BIT13 + BIT12) +#define SOF_STATUS BIT(15) +#define UFRAME (BIT(14) | BIT(13) | BIT(12)) #define FRAME 0x07FF #define USB_ADRS_SHIFT 16 /*--- (0x000C) UTMI Characteristic 1 Register */ -#define SQUSET (BIT07 + BIT06 + BIT05 + BIT04) +#define SQUSET (BIT(7) | BIT(6) | BIT(5) | BIT(4)) -#define USB_SQUSET (BIT06 + BIT05 + BIT04) +#define USB_SQUSET (BIT(6) | BIT(5) | BIT(4)) /*--- (0x0010) TEST Control Register */ -#define FORCEHSBIT02 -#define CS_TESTMODEEN BIT01 -#define LOOPBACK BIT00 +#define FORCEHSBIT(2) +#define CS_TESTMODEEN BIT(1) +#define LOOPBACK BIT(0) /*--- (0x0018) Setup Data 0 Register */ /*--- (0x001C) Setup Data 1 Register */ /*--- (0x0020) USB Interrupt Status Register */ #define EPN_INT0x0000 -#define EP15
Re: [PATCH v2] staging: wfx: fixed misspelled word in comment
On Tue, Aug 04, 2020 at 06:17:47PM +0530, Aditya Bansal wrote:
> From: Aditya Bansal
>
> Subject: [PATCH v2] fixed typo in driver/staging/wfx/hif_tx.c file
>
> Correct the spelling of word function and careful
>
> Signed-off-by: Aditya Bansal
> ---
>
> diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
> index 5110f9b93762..fc12f9dcefce 100644
> --- a/drivers/staging/wfx/hif_tx.c
> +++ b/drivers/staging/wfx/hif_tx.c
> @@ -125,7 +125,7 @@ int wfx_cmd_send(struct wfx_dev *wdev, struct hif_msg
> *request,
>
> // This function is special. After HIF_REQ_ID_SHUT_DOWN, chip won't reply to
> any
> // request anymore. We need to slightly hack struct wfx_hif_cmd for that
> job. Be
> -// carefull to only call this funcion during device unregister.
> +// careful to only call this function during device unregister.
> int hif_shutdown(struct wfx_dev *wdev)
> {
> int ret;
> ___
> devel mailing list
> de...@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Does not apply to my tree, can you please refresh and resend?
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] binder: Modify comments
On Tue, Aug 18, 2020 at 3:34 AM hui yang wrote: > > From: YangHui > > The function name should is binder_alloc_new_buf() > Reviewed-by: Martijn Coenen > Signed-off-by: YangHui > --- > drivers/android/binder_alloc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c > index 6960969..8c98d12 100644 > --- a/drivers/android/binder_alloc.c > +++ b/drivers/android/binder_alloc.c > @@ -652,7 +652,7 @@ static void binder_free_buf_locked(struct binder_alloc > *alloc, > * @alloc: binder_alloc for this proc > * @buffer:kernel pointer to buffer > * > - * Free the buffer allocated via binder_alloc_new_buffer() > + * Free the buffer allocated via binder_alloc_new_buf() > */ > void binder_alloc_free_buf(struct binder_alloc *alloc, > struct binder_buffer *buffer) > -- > 2.7.4 > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: wfx: refactor to avoid duplication at hif_tx.c
On Mon, Aug 10, 2020 at 11:38:33AM +0200, Jérôme Pouiller wrote:
> Hello Tomer,
>
> On Wednesday 5 August 2020 14:14:42 CEST Tomer Samara wrote:
> >
> > Add functions wfx_full_send(), wfx_full_send_no_reply_async(),
> > wfx_full_send_no_reply() and wfx_full_send_no_reply_free()
> > which works as follow:
> > wfx_full_send() - simple wrapper for both wfx_fill_header()
> > and wfx_cmd_send().
> > wfx_full_send_no_reply_async() - wrapper for both but with
> > NULL as reply and size zero.
> > wfx_full_send_no_reply() - same as wfx_full_send_no_reply_async()
> >but with false async value
> > wfx_full_send_no_reply_free() - same as wfx_full_send_no_reply()
> > but also free the struct hif_msg.
> >
> > Signed-off-by: Tomer Samara
> > ---
> > Changes in v2:
> > - Changed these functions to static
> >
> > drivers/staging/wfx/hif_tx.c | 180 ---
> > 1 file changed, 80 insertions(+), 100 deletions(-)
> >
> > diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
> > index 5110f9b93762..17f668fa40a0 100644
> > --- a/drivers/staging/wfx/hif_tx.c
> > +++ b/drivers/staging/wfx/hif_tx.c
> > @@ -40,7 +40,7 @@ static void wfx_fill_header(struct hif_msg *hif, int
> > if_id,
> >
> > static void *wfx_alloc_hif(size_t body_len, struct hif_msg **hif)
> > {
> > - *hif = kzalloc(sizeof(struct hif_msg) + body_len, GFP_KERNEL);
> > + *hif = kzalloc(sizeof(*hif) + body_len, GFP_KERNEL);
>
> This change is not related to the rest of the patch. It should probably be
> split out.
>
> > if (*hif)
> > return (*hif)->body;
> > else
> > @@ -123,9 +123,39 @@ int wfx_cmd_send(struct wfx_dev *wdev, struct hif_msg
> > *request,
> > return ret;
> > }
> >
> > +static int wfx_full_send(struct wfx_dev *wdev, struct hif_msg *hif, void
> > *reply,
> > +size_t reply_len, bool async, int if_id, unsigned
> > int cmd,
> > +int size)
> > +{
> > + wfx_fill_header(hif, if_id, cmd, size);
> > + return wfx_cmd_send(wdev, hif, reply, reply_len, async);
> > +}
>
> This function takes 8 parameters. Are you sure it simplifies the code?
>
> In add, it does two actions: modify hif and send it. I would keep these
> two actions separated.
>
> > +
> > +static int wfx_full_send_no_reply_async(struct wfx_dev *wdev, struct
> > hif_msg *hif, int if_id,
> > + unsigned int cmd, int size, bool
> > async)
> > +{
> > + return wfx_full_send(wdev, hif, NULL, 0, async, if_id, cmd, size);
> > +}
>
> Does it make sense to have a parameter 'async' to
> wfx_full_send_no_reply_async()? It is weird to call this function with
> async=false, no?
>
> > +
> > +static int wfx_full_send_no_reply(struct wfx_dev *wdev, struct hif_msg
> > *hif, int if_id,
> > + unsigned int cmd, int size)
> > +{
> > + return wfx_full_send_no_reply_async(wdev, hif, if_id, cmd, size,
> > false);
> > +}
> > +
> > +static int wfx_full_send_no_reply_free(struct wfx_dev *wdev, struct
> > hif_msg *hif, int if_id,
> > + unsigned int cmd, int size)
> > +{
> > + int ret;
> > +
> > + ret = wfx_full_send_no_reply(wdev, hif, if_id, cmd, size);
> > + kfree(hif);
> > + return ret;
> > +}
>
> One more time, sending the data and releasing are unrelated actions.
> Indeed, it saves a few lines of code, but is it really an improvement?
>
> > +
> > // This function is special. After HIF_REQ_ID_SHUT_DOWN, chip won't reply
> > to any
> > // request anymore. We need to slightly hack struct wfx_hif_cmd for that
> > job. Be
> > -// carefull to only call this funcion during device unregister.
> > +// careful to only call this function during device unregister.
>
> Not related to the rest of the patch.
>
> [...]
>
> As it stands, I think this change does not improve the code. Obviously, it
> is a subjective opinion. What the other developers think about it?
I agree with you, this just makes things more complex for no good
reason.
Now dropped from my review queue.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723bs: os_dep: fix coding style issue in xmit_linux.c
On Mon, Aug 03, 2020 at 12:26:44AM +0530, Mohammed Rushad wrote:
> This is a patch to the xmit_linux.c file that fixes brace and missing
> line warning found by checkpatch.pl tool
>
> Signed-off-by: Mohammed Rushad
> ---
> drivers/staging/rtl8723bs/os_dep/xmit_linux.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
> b/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
> index fec8a8caaa46..b199d355e568 100644
> --- a/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
> +++ b/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
> @@ -148,13 +148,13 @@ static int rtw_mlcst2unicst(struct adapter *padapter,
> struct sk_buff *skb)
> /* free sta asoc_queue */
> while (phead != plist) {
> int stainfo_offset;
> +
> psta = LIST_CONTAINOR(plist, struct sta_info, asoc_list);
> plist = get_next(plist);
>
> stainfo_offset = rtw_stainfo_offset(pstapriv, psta);
> - if (stainfo_offset_valid(stainfo_offset)) {
> + if (stainfo_offset_valid(stainfo_offset))
> chk_alive_list[chk_alive_num++] = stainfo_offset;
> - }
> }
> spin_unlock_bh(&pstapriv->asoc_list_lock);
>
As trivial as it is, this is still two different things in a single
patch :(
If this was the last remaining issue in this file, I might consider it,
but it isn't, so please break up your changes into
one-type-of-change-per-patch.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723bs: os_dep: fix function-name print using __func__
On Wed, Aug 12, 2020 at 09:37:45PM +0530, Mohammed Rushad wrote:
> This patch to the os_intfs.c fixes the printing of function names using
> the preferred '"%s...", __func__' and alignment issues as pointed out by
> the checkpatch.pl tool.
>
> Signed-off-by: Mohammed Rushad
> ---
> drivers/staging/rtl8723bs/os_dep/os_intfs.c | 56 +++--
> 1 file changed, 29 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/os_dep/os_intfs.c
> b/drivers/staging/rtl8723bs/os_dep/os_intfs.c
> index 27f990a01a23..0460db4ae660 100644
> --- a/drivers/staging/rtl8723bs/os_dep/os_intfs.c
> +++ b/drivers/staging/rtl8723bs/os_dep/os_intfs.c
> @@ -400,17 +400,17 @@ u16 rtw_recv_select_queue(struct sk_buff *skb)
> memcpy(ð_type, pdata + (ETH_ALEN << 1), 2);
>
> switch (be16_to_cpu(eth_type)) {
> - case ETH_P_IP:
> + case ETH_P_IP:
>
> - piphdr = (struct iphdr *)(pdata + ETH_HLEN);
> + piphdr = (struct iphdr *)(pdata + ETH_HLEN);
>
> - dscp = piphdr->tos & 0xfc;
> + dscp = piphdr->tos & 0xfc;
>
> - priority = dscp >> 5;
> + priority = dscp >> 5;
>
> - break;
> - default:
> - priority = 0;
> + break;
> + default:
> + priority = 0;
> }
>
> return rtw_1d_to_queue[priority];
> @@ -539,7 +539,7 @@ u32 rtw_start_drv_threads(struct adapter *padapter)
> {
> u32 _status = _SUCCESS;
>
> - RT_TRACE(_module_os_intfs_c_, _drv_info_, ("+rtw_start_drv_threads\n"));
> + RT_TRACE(_module_os_intfs_c_, _drv_info_, ("+%s\n", __func__));
> padapter->xmitThread = kthread_run(rtw_xmit_thread, padapter,
> "RTW_XMIT_THREAD");
> if (IS_ERR(padapter->xmitThread))
> _status = _FAIL;
> @@ -556,7 +556,7 @@ u32 rtw_start_drv_threads(struct adapter *padapter)
>
> void rtw_stop_drv_threads(struct adapter *padapter)
> {
> - RT_TRACE(_module_os_intfs_c_, _drv_info_, ("+rtw_stop_drv_threads\n"));
> + RT_TRACE(_module_os_intfs_c_, _drv_info_, ("+%s\n", __func__));
>
> rtw_stop_cmd_thread(padapter);
>
> @@ -710,7 +710,7 @@ u8 rtw_init_drv_sw(struct adapter *padapter)
> {
> u8 ret8 = _SUCCESS;
>
> - RT_TRACE(_module_os_intfs_c_, _drv_info_, ("+rtw_init_drv_sw\n"));
> + RT_TRACE(_module_os_intfs_c_, _drv_info_, ("+%s\n", __func__));
>
> rtw_init_default_value(padapter);
>
> @@ -773,29 +773,29 @@ u8 rtw_init_drv_sw(struct adapter *padapter)
>
> exit:
>
> - RT_TRACE(_module_os_intfs_c_, _drv_info_, ("-rtw_init_drv_sw\n"));
> + RT_TRACE(_module_os_intfs_c_, _drv_info_, ("-%s\n", __func__));
>
> return ret8;
> }
>
> void rtw_cancel_all_timer(struct adapter *padapter)
> {
> - RT_TRACE(_module_os_intfs_c_, _drv_info_, ("+rtw_cancel_all_timer\n"));
> + RT_TRACE(_module_os_intfs_c_, _drv_info_, ("+%s\n", __func__));
>
> del_timer_sync(&padapter->mlmepriv.assoc_timer);
> - RT_TRACE(_module_os_intfs_c_, _drv_info_, ("rtw_cancel_all_timer:cancel
> association timer complete!\n"));
> + RT_TRACE(_module_os_intfs_c_, _drv_info_, ("%s:cancel association timer
> complete!\n", __func__));
>
> del_timer_sync(&padapter->mlmepriv.scan_to_timer);
> - RT_TRACE(_module_os_intfs_c_, _drv_info_, ("rtw_cancel_all_timer:cancel
> scan_to_timer!\n"));
> + RT_TRACE(_module_os_intfs_c_, _drv_info_, ("%s:cancel
> scan_to_timer!\n", __func__));
>
> del_timer_sync(&padapter->mlmepriv.dynamic_chk_timer);
> - RT_TRACE(_module_os_intfs_c_, _drv_info_, ("rtw_cancel_all_timer:cancel
> dynamic_chk_timer!\n"));
> + RT_TRACE(_module_os_intfs_c_, _drv_info_, ("%s:cancel
> dynamic_chk_timer!\n", __func__));
>
> del_timer_sync(&(adapter_to_pwrctl(padapter)->pwr_state_check_timer));
>
> del_timer_sync(&padapter->mlmepriv.set_scan_deny_timer);
> rtw_clear_scan_deny(padapter);
> - RT_TRACE(_module_os_intfs_c_, _drv_info_, ("rtw_cancel_all_timer:cancel
> set_scan_deny_timer!\n"));
> + RT_TRACE(_module_os_intfs_c_, _drv_info_, ("%s:cancel
> set_scan_deny_timer!\n", __func__));
>
> del_timer_sync(&padapter->recvpriv.signal_stat_timer);
>
> @@ -805,7 +805,7 @@ void rtw_cancel_all_timer(struct adapter *padapter)
>
> u8 rtw_free_drv_sw(struct adapter *padapter)
> {
> - RT_TRACE(_module_os_intfs_c_, _drv_info_, ("==>rtw_free_drv_sw"));
> + RT_TRACE(_module_os_intfs_c_, _drv_info_, ("==>%s", __func__));
>
> free_mlme_ext_priv(&padapter->mlmeextpriv);
>
> @@ -829,7 +829,7 @@ u8 rtw_free_drv_sw(struct adapter *padapter)
>
> rtw_hal_free_data(padapter);
>
> - RT_TRACE(_module_os_intfs_c_, _drv_info_, ("<==rtw_free_drv_sw\n"));
> + RT_TRACE(_module_os_intfs_c_, _drv_info_, ("<==%s\n", __func__));
>
> /* free the old_pnetdev */
> if (padapter->rereg_nd_name_priv.old_pnetdev) {
> @@ -841,7 +841,7
Re: [PATCH v2 2/4] staging: android: Add error handling to ion_page_pool_shrink
On Sun, Aug 16, 2020 at 10:24:22PM +0300, Tomer Samara wrote: > Add error check to ion_page_pool_shrink after calling > ion_page_pool_remove, due to converting BUG_ON to WARN_ON. > > Signed-off-by: Tomer Samara So this fixes a previous patch? That's not good, please merge them together so you do not cause a bug and then fix it up later on. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/4] staging: android: Replace BUG_ON with WARN_ON
On Sun, Aug 16, 2020 at 10:23:25PM +0300, Tomer Samara wrote:
> BUG_ON() is replaced with WARN_ON at ion_page_pool.c
Why?
> Fixes the following issue:
> Avoid crashing the kernel - try using WARN_ON & recovery code ratherthan
> BUG() or BUG_ON().
Ideally you can get rid of WARN_ON() too, right?
Many systems run in panic-on-warn mode, so this really does not change
anything. Try fixing this up properly to not crash at all.
>
> Signed-off-by: Tomer Samara
> ---
> drivers/staging/android/ion/ion_page_pool.c | 12
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/android/ion/ion_page_pool.c
> b/drivers/staging/android/ion/ion_page_pool.c
> index 0198b886d906..c1b9eda35c96 100644
> --- a/drivers/staging/android/ion/ion_page_pool.c
> +++ b/drivers/staging/android/ion/ion_page_pool.c
> @@ -46,11 +46,13 @@ static struct page *ion_page_pool_remove(struct
> ion_page_pool *pool, bool high)
> struct page *page;
>
> if (high) {
> - BUG_ON(!pool->high_count);
> + if (WARN_ON(!pool->high_count))
> + return NULL;
And can you test this that it works properly?
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 3/4] staging: android: Convert BUG to WARN
On Sun, Aug 16, 2020 at 10:30:10PM +0300, Tomer Samara wrote: > replace BUG() with WARN() at ion_sytem_heap.c, this > fix the following checkpatch issue: > Avoid crashing the kernel - try using WARN_ON & > recovery code ratherthan BUG() or BUG_ON(). > > Signed-off-by: Tomer Samara > --- > drivers/staging/android/ion/ion_system_heap.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/android/ion/ion_system_heap.c > b/drivers/staging/android/ion/ion_system_heap.c > index eac0632ab4e8..37065a59ca69 100644 > --- a/drivers/staging/android/ion/ion_system_heap.c > +++ b/drivers/staging/android/ion/ion_system_heap.c > @@ -30,7 +30,8 @@ static int order_to_index(unsigned int order) > for (i = 0; i < NUM_ORDERS; i++) > if (order == orders[i]) > return i; > - BUG(); > + > + WARN(1, "%s: Did not found index to order %d", __FUNCTION__, order); Same question as before, I think this didn't really change anything :( ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 4/4] staging: android: Add error handling to order_to_index callers
On Sun, Aug 16, 2020 at 10:31:22PM +0300, Tomer Samara wrote: > Add error check to: > - free_buffer_page > - alloc_buffer_page > after calling order_to_index, due to converting BUG to WARN at > order_to_index. You are fixing a bug you caused in a previous patch, not good :) thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 00/44] SPMI patches needed by Hikey 970
On Mon, Aug 17, 2020 at 09:10:19AM +0200, Mauro Carvalho Chehab wrote: > Hi Greg, > > This patch series is part of a work I'm doing in order to be able to support > a HiKey 970 board that I recently got on my hands. With this applied, I get the following build error: ERROR: modpost: "__spmi_driver_register" [drivers/staging/hikey9xx/hi6421-spmi-pmic.ko] undefined! ERROR: modpost: "spmi_ext_register_writel" [drivers/staging/hikey9xx/hi6421-spmi-pmic.ko] undefined! ERROR: modpost: "spmi_ext_register_readl" [drivers/staging/hikey9xx/hi6421-spmi-pmic.ko] undefined! ERROR: modpost: "spmi_controller_add" [drivers/staging/hikey9xx/hisi-spmi-controller.ko] undefined! ERROR: modpost: "spmi_controller_alloc" [drivers/staging/hikey9xx/hisi-spmi-controller.ko] undefined! ERROR: modpost: "spmi_controller_remove" [drivers/staging/hikey9xx/hisi-spmi-controller.ko] undefined! I'll take this in my testing tree for now, can you send a follow-on patch to fix this? And I only took the first 41 patches in this series, see my comments on the rest. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/4] staging: android: Replace BUG_ON with WARN_ON
On Tue, Aug 18, 2020 at 04:11:06PM +0200, Greg Kroah-Hartman wrote:
> On Sun, Aug 16, 2020 at 10:23:25PM +0300, Tomer Samara wrote:
> > BUG_ON() is replaced with WARN_ON at ion_page_pool.c
>
> Why?
>
> > Fixes the following issue:
> > Avoid crashing the kernel - try using WARN_ON & recovery code ratherthan
> > BUG() or BUG_ON().
>
> Ideally you can get rid of WARN_ON() too, right?
>
> Many systems run in panic-on-warn mode, so this really does not change
> anything. Try fixing this up properly to not crash at all.
>
You mean by that to just remove the WARN_ON and leave the condition the
same?
> >
> > Signed-off-by: Tomer Samara
> > ---
> > drivers/staging/android/ion/ion_page_pool.c | 12
> > 1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/staging/android/ion/ion_page_pool.c
> > b/drivers/staging/android/ion/ion_page_pool.c
> > index 0198b886d906..c1b9eda35c96 100644
> > --- a/drivers/staging/android/ion/ion_page_pool.c
> > +++ b/drivers/staging/android/ion/ion_page_pool.c
> > @@ -46,11 +46,13 @@ static struct page *ion_page_pool_remove(struct
> > ion_page_pool *pool, bool high)
> > struct page *page;
> >
> > if (high) {
> > - BUG_ON(!pool->high_count);
> > + if (WARN_ON(!pool->high_count))
> > + return NULL;
>
> And can you test this that it works properly?
>
> thanks,
>
> greg k-h
Will do :)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/4] staging: android: Replace BUG_ON with WARN_ON
On Tue, Aug 18, 2020 at 05:19:40PM +0300, Tomer Samara wrote: > On Tue, Aug 18, 2020 at 04:11:06PM +0200, Greg Kroah-Hartman wrote: > > On Sun, Aug 16, 2020 at 10:23:25PM +0300, Tomer Samara wrote: > > > BUG_ON() is replaced with WARN_ON at ion_page_pool.c > > > > Why? > > > > > Fixes the following issue: > > > Avoid crashing the kernel - try using WARN_ON & recovery code ratherthan > > > BUG() or BUG_ON(). > > > > Ideally you can get rid of WARN_ON() too, right? > > > > Many systems run in panic-on-warn mode, so this really does not change > > anything. Try fixing this up properly to not crash at all. > > > You mean by that to just remove the WARN_ON and leave the condition the > same? Or fix the problem that could ever cause this check to fire. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 00/44] SPMI patches needed by Hikey 970
Em Tue, 18 Aug 2020 16:17:50 +0200 Greg Kroah-Hartman escreveu: > On Mon, Aug 17, 2020 at 09:10:19AM +0200, Mauro Carvalho Chehab wrote: > > Hi Greg, > > > > This patch series is part of a work I'm doing in order to be able to support > > a HiKey 970 board that I recently got on my hands. > > With this applied, I get the following build error: > ERROR: modpost: "__spmi_driver_register" > [drivers/staging/hikey9xx/hi6421-spmi-pmic.ko] undefined! > ERROR: modpost: "spmi_ext_register_writel" > [drivers/staging/hikey9xx/hi6421-spmi-pmic.ko] undefined! > ERROR: modpost: "spmi_ext_register_readl" > [drivers/staging/hikey9xx/hi6421-spmi-pmic.ko] undefined! > ERROR: modpost: "spmi_controller_add" > [drivers/staging/hikey9xx/hisi-spmi-controller.ko] undefined! > ERROR: modpost: "spmi_controller_alloc" > [drivers/staging/hikey9xx/hisi-spmi-controller.ko] undefined! > ERROR: modpost: "spmi_controller_remove" > [drivers/staging/hikey9xx/hisi-spmi-controller.ko] undefined! > > > I'll take this in my testing tree for now, can you send a follow-on > patch to fix this? Surely. That's because it got moved from drivers/spmi/Kconfig. The Kconfig var was inside a: if SPMI ... endif This driver should "depends on SPMI". I'll send you a patch in a few. Thanks, Mauro ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: emxx_udc: Use standard BIT() macro
On 8/18/20 6:49 AM, Alex Dewar wrote: > Currently emxx_udc.h defines bit values using local macros. Use the > standard one instead. > > Also, combine bit values with bitwise-or rather than addition, as > suggested by Coccinelle. > > Signed-off-by: Alex Dewar Hi, Does this build? Just checking. Looks like it would need this: #include since it (indirectly) provides definition of the BIT() macro. > --- > drivers/staging/emxx_udc/emxx_udc.h | 456 +--- > 1 file changed, 211 insertions(+), 245 deletions(-) thanks. -- ~Randy ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
On 2020-08-17 08:49, Mauro Carvalho Chehab wrote: Add a driver for the Kirin 960/970 iommu. As on the past series, this starts from the original 4.9 driver from the 96boards tree: https://github.com/96boards-hikey/linux/tree/hikey970-v4.9 The remaining patches add SPDX headers and make it build and run with the upstream Kernel. Chenfeng (1): iommu: add support for HiSilicon Kirin 960/970 iommu Mauro Carvalho Chehab (15): iommu: hisilicon: remove default iommu_map_sg handler iommu: hisilicon: map and unmap ops gained new arguments iommu: hisi_smmu_lpae: rebase it to work with upstream iommu: hisi_smmu: remove linux/hisi/hisi-iommu.h iommu: hisilicon: cleanup its code style iommu: hisi_smmu_lpae: get rid of IOMMU_SEC and IOMMU_DEVICE iommu: get rid of map/unmap tile functions iommu: hisi_smmu_lpae: use the right code to get domain-priv data iommu: hisi_smmu_lpae: convert it to probe_device iommu: add Hisilicon Kirin970 iommu at the building system iommu: hisi_smmu_lpae: cleanup printk macros iommu: hisi_smmu_lpae: make OF compatible more standard Echoing the other comments about none of the driver patches being CC'd to the IOMMU list... Still, I dug the series up on lore and frankly I'm not sure what to make of it - AFAICS the "driver" is just yet another implementation of Arm LPAE pagetable code, with no obvious indication of how those pagetables ever get handed off to IOMMU hardware (and indeed no indication of IOMMU hardware at all). Can you explain how it's supposed to work? And as a pre-emptive strike, we really don't need any more LPAE implementations - that's what the io-pgtable library is all about (which incidentally has been around since 4.0...). I think that should make the issue of preserving authorship largely moot since there's no need to preserve most of the code anyway ;) Robin. dt: add an spec for the Kirin36x0 SMMU dt: hi3670-hikey970.dts: load the SMMU driver on Hikey970 staging: hikey9xx: add an item about the iommu driver .../iommu/hisilicon,kirin36x0-smmu.yaml | 55 ++ .../boot/dts/hisilicon/hi3670-hikey970.dts| 3 + drivers/staging/hikey9xx/Kconfig | 9 + drivers/staging/hikey9xx/Makefile | 1 + drivers/staging/hikey9xx/TODO | 1 + drivers/staging/hikey9xx/hisi_smmu.h | 196 ++ drivers/staging/hikey9xx/hisi_smmu_lpae.c | 648 ++ 7 files changed, 913 insertions(+) create mode 100644 Documentation/devicetree/bindings/iommu/hisilicon,kirin36x0-smmu.yaml create mode 100644 drivers/staging/hikey9xx/hisi_smmu.h create mode 100644 drivers/staging/hikey9xx/hisi_smmu_lpae.c ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/6] staging: mfd: hi6421-spmi-pmic: get rid of interrupt properties
Both irqnum and irqarray properties reflect the same thing:
the number of bits and bytes for interrupts at this
chipset. E. g.:
irqnum = 8 x irqarray
This can be seen by the way pending interrupts are handled:
/* During probe time */
pmic->irqs = devm_kzalloc(dev, pmic->irqnum * sizeof(int), GFP_KERNEL);
/* While handling IRQs */
for (i = 0; i < pmic->irqarray; i++) {
pending = hi6421_spmi_pmic_read(pmic, (i + pmic->irq_addr));
pending &= 0xff;
for_each_set_bit(offset, &pending, 8)
generic_handle_irq(pmic->irqs[offset + i * 8]);
}
Going further, there are some logic at the driver which assumes
that irqarray is 2:
/* solve powerkey order */
if ((i == HISI_IRQ_KEY_NUM) &&
((pending & HISI_IRQ_KEY_VALUE) == HISI_IRQ_KEY_VALUE)) {
generic_handle_irq(pmic->irqs[HISI_IRQ_KEY_DOWN]);
generic_handle_irq(pmic->irqs[HISI_IRQ_KEY_UP]);
pending &= (~HISI_IRQ_KEY_VALUE);
}
As HISI_IRQ_KEY_DOWN and HISI_IRQ_KEY_UP are fixed values
and don't depend on irqnum/irqarray.
The IRQ addr and mask addr seem to be also fixed, based on some
comments at the OF parsing code. So, get rid of them too,
removing the of parsing function completely.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/staging/hikey9xx/hi6421-spmi-pmic.c | 80 ++---
include/linux/mfd/hi6421-spmi-pmic.h| 15
2 files changed, 20 insertions(+), 75 deletions(-)
diff --git a/drivers/staging/hikey9xx/hi6421-spmi-pmic.c
b/drivers/staging/hikey9xx/hi6421-spmi-pmic.c
index 9d73458ca65a..7817c0637737 100644
--- a/drivers/staging/hikey9xx/hi6421-spmi-pmic.c
+++ b/drivers/staging/hikey9xx/hi6421-spmi-pmic.c
@@ -38,6 +38,12 @@
/* 8-bit register offset in PMIC */
#define HISI_MASK_STATE0xff
+#define HISI_IRQ_ARRAY 2
+#define HISI_IRQ_NUM (HISI_IRQ_ARRAY * 8)
+
+#define SOC_PMIC_IRQ_MASK_0_ADDR 0x0202
+#define SOC_PMIC_IRQ0_ADDR 0x0212
+
#define HISI_IRQ_KEY_NUM 0
#define HISI_IRQ_KEY_VALUE 0xc0
#define HISI_IRQ_KEY_DOWN 7
@@ -121,13 +127,13 @@ static irqreturn_t hi6421_spmi_irq_handler(int irq, void
*data)
unsigned long pending;
int i, offset;
- for (i = 0; i < pmic->irqarray; i++) {
- pending = hi6421_spmi_pmic_read(pmic, (i + pmic->irq_addr));
+ for (i = 0; i < HISI_IRQ_ARRAY; i++) {
+ pending = hi6421_spmi_pmic_read(pmic, (i + SOC_PMIC_IRQ0_ADDR));
pending &= HISI_MASK_FIELD;
if (pending != 0)
pr_debug("pending[%d]=0x%lx\n\r", i, pending);
- hi6421_spmi_pmic_write(pmic, (i + pmic->irq_addr), pending);
+ hi6421_spmi_pmic_write(pmic, (i + SOC_PMIC_IRQ0_ADDR), pending);
/* solve powerkey order */
if ((i == HISI_IRQ_KEY_NUM) &&
@@ -153,7 +159,7 @@ static void hi6421_spmi_irq_mask(struct irq_data *d)
unsigned long flags;
offset = (irqd_to_hwirq(d) >> 3);
- offset += pmic->irq_mask_addr;
+ offset += SOC_PMIC_IRQ_MASK_0_ADDR;
spin_lock_irqsave(&pmic->lock, flags);
data = hi6421_spmi_pmic_read(pmic, offset);
@@ -169,7 +175,7 @@ static void hi6421_spmi_irq_unmask(struct irq_data *d)
unsigned long flags;
offset = (irqd_to_hwirq(d) >> 3);
- offset += pmic->irq_mask_addr;
+ offset += SOC_PMIC_IRQ_MASK_0_ADDR;
spin_lock_irqsave(&pmic->lock, flags);
data = hi6421_spmi_pmic_read(pmic, offset);
@@ -204,60 +210,20 @@ static const struct irq_domain_ops hi6421_spmi_domain_ops
= {
.xlate = irq_domain_xlate_twocell,
};
-static int get_pmic_device_tree_data(struct device_node *np,
-struct hi6421_spmi_pmic *pmic)
-{
- int ret = 0;
-
- /* IRQ number */
- ret = of_property_read_u32(np, "irq-num", &pmic->irqnum);
- if (ret) {
- pr_err("no irq-num property set\n");
- ret = -ENODEV;
- return ret;
- }
-
- /* Size of IRQ array */
- ret = of_property_read_u32(np, "irq-array", &pmic->irqarray);
- if (ret) {
- pr_err("no irq-array property set\n");
- ret = -ENODEV;
- return ret;
- }
-
- /* SOC_PMIC_IRQ_MASK_0_ADDR */
- ret = of_property_read_u32(np, "irq-mask-addr", &pmic->irq_mask_addr);
- if (ret) {
- pr_err("no irq-mask-addr property set\n");
- ret = -ENODEV;
- return ret;
- }
-
- /* SOC_PMIC_IRQ0_ADDR */
- ret = of_property_read_u32(np, "irq-addr", &pmic->irq_addr);
- if (ret) {
- pr_err("no irq-addr property set\n");
- ret = -ENODEV;
- return ret;
- }
-
[PATCH 1/6] staging: hikey9xx: fix Kconfig dependency chain
Both the SPMI controller and the SPMI PMIC driver depends on the SPMI bus support. The dependency for the regulator is also wrong: it should depends on the SPMI version of the HiSilicon 6421, and not on the normal one. Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/hikey9xx/Kconfig | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/staging/hikey9xx/Kconfig b/drivers/staging/hikey9xx/Kconfig index 76267b9be562..a004839e8fa9 100644 --- a/drivers/staging/hikey9xx/Kconfig +++ b/drivers/staging/hikey9xx/Kconfig @@ -5,6 +5,7 @@ config SPMI_HISI3670 tristate "Hisilicon 3670 SPMI Controller" select IRQ_DOMAIN_HIERARCHY depends on HAS_IOMEM + depends on SPMI help If you say yes to this option, support will be included for the built-in SPMI PMIC Arbiter interface on Hisilicon 3670 @@ -14,6 +15,7 @@ config SPMI_HISI3670 config MFD_HI6421_SPMI tristate "HiSilicon Hi6421v600 SPMI PMU/Codec IC" depends on OF + depends on SPMI select MFD_CORE help Add support for HiSilicon Hi6421v600 SPMI PMIC. Hi6421 includes @@ -28,7 +30,7 @@ config MFD_HI6421_SPMI # to be placed at drivers/regulator config REGULATOR_HI6421V600 tristate "HiSilicon Hi6421v600 PMIC voltage regulator support" - depends on MFD_HI6421_PMIC && OF + depends on MFD_HI6421_SPMI && OF help This driver provides support for the voltage regulators on HiSilicon Hi6421v600 PMU / Codec IC. -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/6] staging: spmi: hisi-spmi-controller: change compatible string
Add the chipset name at the compatible string, as other
HiSilicon chipsets with SPMI bus might require something
different.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/staging/hikey9xx/hisi-spmi-controller.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/hikey9xx/hisi-spmi-controller.c
b/drivers/staging/hikey9xx/hisi-spmi-controller.c
index 513d962b8bce..66a0b296e06f 100644
--- a/drivers/staging/hikey9xx/hisi-spmi-controller.c
+++ b/drivers/staging/hikey9xx/hisi-spmi-controller.c
@@ -324,7 +324,8 @@ static int spmi_del_controller(struct platform_device *pdev)
}
static const struct of_device_id spmi_controller_match_table[] = {
- { .compatible = "hisilicon,spmi-controller",
+ {
+ .compatible = "hisilicon,kirin970-spmi-controller",
},
{}
};
--
2.26.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/6] Do some cleanups at the HiSilicon 6421 regulator
Hi Greg, Patch 1 contains a Kconfig fixup, addressing the problem you noticed when building those drivers; Patches 2 to 4 addresses some issues at the device tree, pointed by Rob's review. Patch 5 adds the DT documentation. As requested, I moved those also to the staging dir. Patch 6 adds an entry at MAINTAINERS. I changed the ML to point to the devel ML. Thanks! Mauro Mauro Carvalho Chehab (6): staging: hikey9xx: fix Kconfig dependency chain staging: mfd: hi6421-spmi-pmic: get rid of interrupt properties staging: spmi: hisi-spmi-controller: change compatible string staging: mfd: hi6421-spmi-pmic: Simplify the compatible string dt: document HiSilicon SPMI controller and mfd/regulator properties MAINTAINERS: add an entry for HiSilicon 6421v600 drivers MAINTAINERS | 6 + drivers/staging/hikey9xx/Kconfig | 4 +- drivers/staging/hikey9xx/hi6421-spmi-pmic.c | 82 +++-- .../staging/hikey9xx/hisi-spmi-controller.c | 3 +- .../hikey9xx/hisilicon,hi6421-spmi-pmic.yaml | 159 ++ .../hisilicon,hisi-spmi-controller.yaml | 62 +++ include/linux/mfd/hi6421-spmi-pmic.h | 15 -- 7 files changed, 253 insertions(+), 78 deletions(-) create mode 100644 drivers/staging/hikey9xx/hisilicon,hi6421-spmi-pmic.yaml create mode 100644 drivers/staging/hikey9xx/hisilicon,hisi-spmi-controller.yaml -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/6] staging: mfd: hi6421-spmi-pmic: Simplify the compatible string
It is clear that this driver is for PMIC. So, get rid of
it at the compatible.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/staging/hikey9xx/hi6421-spmi-pmic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/hikey9xx/hi6421-spmi-pmic.c
b/drivers/staging/hikey9xx/hi6421-spmi-pmic.c
index 7817c0637737..64b30d263c8d 100644
--- a/drivers/staging/hikey9xx/hi6421-spmi-pmic.c
+++ b/drivers/staging/hikey9xx/hi6421-spmi-pmic.c
@@ -323,7 +323,7 @@ static void hi6421_spmi_pmic_remove(struct spmi_device
*pdev)
}
static const struct of_device_id pmic_spmi_id_table[] = {
- { .compatible = "hisilicon,hi6421-spmi-pmic" },
+ { .compatible = "hisilicon,hi6421-spmi" },
{ }
};
MODULE_DEVICE_TABLE(of, pmic_spmi_id_table);
--
2.26.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/6] dt: document HiSilicon SPMI controller and mfd/regulator properties
Add documentation for the properties needed by the HiSilicon
6421v600 driver, and by the SPMI controller used to access
the chipset.
Signed-off-by: Mauro Carvalho Chehab
---
.../hikey9xx/hisilicon,hi6421-spmi-pmic.yaml | 159 ++
.../hisilicon,hisi-spmi-controller.yaml | 62 +++
2 files changed, 221 insertions(+)
create mode 100644 drivers/staging/hikey9xx/hisilicon,hi6421-spmi-pmic.yaml
create mode 100644 drivers/staging/hikey9xx/hisilicon,hisi-spmi-controller.yaml
diff --git a/drivers/staging/hikey9xx/hisilicon,hi6421-spmi-pmic.yaml
b/drivers/staging/hikey9xx/hisilicon,hi6421-spmi-pmic.yaml
new file mode 100644
index ..c76093f320f1
--- /dev/null
+++ b/drivers/staging/hikey9xx/hisilicon,hi6421-spmi-pmic.yaml
@@ -0,0 +1,159 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/hisilicon,hi6421-spmi-pmic.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: HiSilicon 6421v600 SPMI PMIC
+
+maintainers:
+ - Mauro Carvalho Chehab
+
+description: |
+ HiSilicon 6421v600 should be connected inside a MIPI System Power Management
+ (SPMI) bus. It provides interrupts and power supply.
+
+ The GPIO and interrupt settings are represented as part of the top-level PMIC
+ node.
+
+ The SPMI controller part is provided by
+ Documentation/devicetree/bindings/spmi/hisilicon,hisi-spmi-controller.yaml.
+
+properties:
+ $nodename:
+pattern: "pmic@[0-9a-f]"
+
+ compatible:
+const: hisilicon,hi6421v600-spmi
+
+ reg:
+maxItems: 1
+
+ '#interrupt-cells':
+const: 2
+
+ interrupt-controller:
+description:
+ Identify that the PMIC is capable of behaving as an interrupt controller.
+
+ gpios:
+maxItems: 1
+
+ regulators:
+type: object
+
+properties:
+ '#address-cells':
+const: 1
+
+ '#size-cells':
+const: 0
+
+patternProperties:
+ '^ldo[0-9]+@[0-9a-f]$':
+type: object
+
+$ref: "/schemas/regulator/regulator.yaml#"
+
+properties:
+ reg:
+description: Enable register.
+
+ '#address-cells':
+const: 1
+
+ '#size-cells':
+const: 0
+
+ vsel-reg:
+description: Voltage selector register.
+
+ enable-mask:
+description: Bitmask used to enable the regulator.
+
+ voltage-table:
+description: Table with the selector items for the voltage
regulator.
+minItems: 2
+maxItems: 16
+
+ off-on-delay-us:
+description: Time required for changing state to enabled in
microseconds.
+
+ startup-delay-us:
+description: Startup time in microseconds.
+
+ idle-mode-mask:
+description: Bitmask used to put the regulator on idle mode.
+
+ eco-microamp:
+description: Maximum current while on idle mode.
+
+required:
+ - reg
+ - vsel-reg
+ - enable-mask
+ - voltage-table
+ - off-on-delay-us
+ - startup-delay-us
+
+required:
+ - compatible
+ - reg
+ - regulators
+
+examples:
+ - |
+/* pmic properties */
+
+pmic: pmic@0 {
+ compatible = "hisilicon,hi6421-spmi";
+ reg = <0 0>;
+
+ #interrupt-cells = <2>;
+ interrupt-controller;
+ gpios = <&gpio28 0 0>;
+
+ regulators {
+#address-cells = <1>;
+#size-cells = <0>;
+
+ldo3: ldo3@16 {
+ reg = <0x16>;
+ vsel-reg = <0x51>;
+
+ regulator-name = "ldo3";
+ regulator-min-microvolt = <150>;
+ regulator-max-microvolt = <200>;
+ regulator-boot-on;
+
+ enable-mask = <0x01>;
+
+ voltage-table = <150>, <155>, <160>, <165>,
+ <170>, <1725000>, <175>, <1775000>,
+ <180>, <1825000>, <185>, <1875000>,
+ <190>, <1925000>, <195>, <200>;
+ off-on-delay-us = <2>;
+ startup-delay-us = <120>;
+};
+
+ldo4: ldo4@17 { /* 40 PIN */
+ reg = <0x17>;
+ vsel-reg = <0x52>;
+
+ regulator-name = "ldo4";
+ regulator-min-microvolt = <1725000>;
+ regulator-max-microvolt = <190>;
+ regulator-boot-on;
+
+ enable-mask = <0x01>;
+ idle-mode-mask = <0x10>;
+ eco-microamp = <1>;
+
+ hi6421-vsel = <0x52 0x07>;
+ voltage-table = <1725000>, <175>, <1775000>, <180>,
+ <1825000>, <185>, <1875000>, <190>;
+ off-on-delay-us = <2>;
+ startup-delay-us = <120>;
+};
+ };
+};
diff --git a/drivers/staging/hikey9xx/hisilicon,hisi-spmi-controller.yaml
b/drivers/staging/hikey9xx/hisilicon,hisi-spmi-controller.yaml
new file mode 100644
index ..b1cfa9c3aca6
--- /dev/null
+++ b/drivers/stagin
Re: [PATCH 1/6] staging: hikey9xx: fix Kconfig dependency chain
On Tue, Aug 18, 2020 at 04:58:53PM +0200, Mauro Carvalho Chehab wrote: > Both the SPMI controller and the SPMI PMIC driver > depends on the SPMI bus support. > > The dependency for the regulator is also wrong: > it should depends on the SPMI version of the HiSilicon 6421, > and not on the normal one. > > Signed-off-by: Mauro Carvalho Chehab > --- > drivers/staging/hikey9xx/Kconfig | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/hikey9xx/Kconfig > b/drivers/staging/hikey9xx/Kconfig > index 76267b9be562..a004839e8fa9 100644 > --- a/drivers/staging/hikey9xx/Kconfig > +++ b/drivers/staging/hikey9xx/Kconfig > @@ -5,6 +5,7 @@ config SPMI_HISI3670 > tristate "Hisilicon 3670 SPMI Controller" > select IRQ_DOMAIN_HIERARCHY > depends on HAS_IOMEM > + depends on SPMI > help > If you say yes to this option, support will be included for the > built-in SPMI PMIC Arbiter interface on Hisilicon 3670 > @@ -14,6 +15,7 @@ config SPMI_HISI3670 > config MFD_HI6421_SPMI > tristate "HiSilicon Hi6421v600 SPMI PMU/Codec IC" > depends on OF > + depends on SPMI > select MFD_CORE > help > Add support for HiSilicon Hi6421v600 SPMI PMIC. Hi6421 includes > @@ -28,7 +30,7 @@ config MFD_HI6421_SPMI > # to be placed at drivers/regulator > config REGULATOR_HI6421V600 > tristate "HiSilicon Hi6421v600 PMIC voltage regulator support" > - depends on MFD_HI6421_PMIC && OF > + depends on MFD_HI6421_SPMI && OF > help > This driver provides support for the voltage regulators on > HiSilicon Hi6421v600 PMU / Codec IC. Better, but now I get the following build error: ERROR: modpost: "regulator_map_voltage_iterate" [drivers/staging/hikey9xx/hi6421v600-regulator.ko] undefined! ERROR: modpost: "regulator_list_voltage_table" [drivers/staging/hikey9xx/hi6421v600-regulator.ko] undefined! ERROR: modpost: "of_get_regulator_init_data" [drivers/staging/hikey9xx/hi6421v600-regulator.ko] undefined! ERROR: modpost: "regulator_register" [drivers/staging/hikey9xx/hi6421v600-regulator.ko] undefined! ERROR: modpost: "regulator_unregister" [drivers/staging/hikey9xx/hi6421v600-regulator.ko] undefined! ERROR: modpost: "rdev_get_drvdata" [drivers/staging/hikey9xx/hi6421v600-regulator.ko] undefined! Someone need CONFIG_REGULATOR enabled? Another follow-on patch? :) thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: emxx_udc: Use standard BIT() macro
On Tue, Aug 18, 2020 at 07:29:02AM -0700, Randy Dunlap wrote: > On 8/18/20 6:49 AM, Alex Dewar wrote: > > Currently emxx_udc.h defines bit values using local macros. Use the > > standard one instead. > > > > Also, combine bit values with bitwise-or rather than addition, as > > suggested by Coccinelle. > > > > Signed-off-by: Alex Dewar > > Hi, > > Does this build? Just checking. > > Looks like it would need this: > > #include > > since it (indirectly) provides definition of the BIT() macro. Yeah, it builds, because emxx_udc.c includes emxx_udc.h after a bunch of standard headers. I agree that it would probably be cleaner to have the include in there explicitly, though. Best, Alex > > > --- > > drivers/staging/emxx_udc/emxx_udc.h | 456 +--- > > 1 file changed, 211 insertions(+), 245 deletions(-) > > > thanks. > -- > ~Randy > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: emxx_udc: Allow for building on !ARM
Currently the module can only be test built on ARM, although it seems to build fine on x86. Change this to allow for broader test coverage. Signed-off-by: Alex Dewar --- drivers/staging/emxx_udc/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/emxx_udc/Kconfig b/drivers/staging/emxx_udc/Kconfig index ad1478c53e9e..e7a95b3b6a2f 100644 --- a/drivers/staging/emxx_udc/Kconfig +++ b/drivers/staging/emxx_udc/Kconfig @@ -1,7 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 config USB_EMXX tristate "EMXX USB Function Device Controller" - depends on USB_GADGET && (ARCH_RENESAS || (ARM && COMPILE_TEST)) + depends on USB_GADGET && (ARCH_RENESAS || COMPILE_TEST) help The Emma Mobile series of SoCs from Renesas Electronics and former NEC Electronics include USB Function hardware. -- 2.28.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
Hi Robin,
Em Tue, 18 Aug 2020 15:47:55 +0100
Robin Murphy escreveu:
> On 2020-08-17 08:49, Mauro Carvalho Chehab wrote:
> > Add a driver for the Kirin 960/970 iommu.
> >
> > As on the past series, this starts from the original 4.9 driver from
> > the 96boards tree:
> >
> > https://github.com/96boards-hikey/linux/tree/hikey970-v4.9
> >
> > The remaining patches add SPDX headers and make it build and run with
> > the upstream Kernel.
> >
> > Chenfeng (1):
> >iommu: add support for HiSilicon Kirin 960/970 iommu
> >
> > Mauro Carvalho Chehab (15):
> >iommu: hisilicon: remove default iommu_map_sg handler
> >iommu: hisilicon: map and unmap ops gained new arguments
> >iommu: hisi_smmu_lpae: rebase it to work with upstream
> >iommu: hisi_smmu: remove linux/hisi/hisi-iommu.h
> >iommu: hisilicon: cleanup its code style
> >iommu: hisi_smmu_lpae: get rid of IOMMU_SEC and IOMMU_DEVICE
> >iommu: get rid of map/unmap tile functions
> >iommu: hisi_smmu_lpae: use the right code to get domain-priv data
> >iommu: hisi_smmu_lpae: convert it to probe_device
> >iommu: add Hisilicon Kirin970 iommu at the building system
> >iommu: hisi_smmu_lpae: cleanup printk macros
> >iommu: hisi_smmu_lpae: make OF compatible more standard
>
> Echoing the other comments about none of the driver patches being CC'd
> to the IOMMU list...
>
> Still, I dug the series up on lore and frankly I'm not sure what to make
> of it - AFAICS the "driver" is just yet another implementation of Arm
> LPAE pagetable code, with no obvious indication of how those pagetables
> ever get handed off to IOMMU hardware (and indeed no indication of IOMMU
> hardware at all). Can you explain how it's supposed to work?
>
> And as a pre-emptive strike, we really don't need any more LPAE
> implementations - that's what the io-pgtable library is all about (which
> incidentally has been around since 4.0...). I think that should make the
> issue of preserving authorship largely moot since there's no need to
> preserve most of the code anyway ;)
I didn't know about that, since I got a Hikey 970 board for the first time
about one month ago, and that's the first time I looked into iommu code.
My end goal with this is to make the DRM/KMS driver to work with upstream
Kernels.
The full patch series are at:
https://github.com/mchehab/linux/commits/hikey970/to_upstream-2.0-v1.1
(I need to put a new version there, after some changes due to recent
upstream discussions at the regulator's part of the code)
Basically, the DT binding has this, for IOMMU:
smmu_lpae {
compatible = "hisilicon,smmu-lpae";
};
...
dpe: dpe@e860 {
compatible = "hisilicon,kirin970-dpe";
memory-region = <&drm_dma_reserved>;
...
iommu_info {
start-addr = <0x8000>;
size = <0xbfff8000>;
};
}
This is used by kirin9xx_drm_dss.c in order to enable and use
the iommu:
static int dss_enable_iommu(struct platform_device *pdev, struct
dss_hw_ctx *ctx)
{
struct device *dev = NULL;
dev = &pdev->dev;
/* create iommu domain */
ctx->mmu_domain = iommu_domain_alloc(dev->bus);
if (!ctx->mmu_domain) {
pr_err("iommu_domain_alloc failed!\n");
return -EINVAL;
}
iommu_attach_device(ctx->mmu_domain, dev);
return 0;
}
The only place where the IOMMU domain is used is on this part of the
code(error part simplified here) [1]:
void hisi_dss_smmu_on(struct dss_hw_ctx *ctx)
{
uint64_t fama_phy_pgd_base;
uint32_t phy_pgd_base;
...
fama_phy_pgd_base = iommu_iova_to_phys(ctx->mmu_domain, 0);
phy_pgd_base = (uint32_t)fama_phy_pgd_base;
if (WARN_ON(!phy_pgd_base))
return;
set_reg(smmu_base + SMMU_CB_TTBR0, phy_pgd_base, 32, 0);
}
[1]
https://github.com/mchehab/linux/commit/36da105e719b47bbe9d6cb7e5619b30c7f3eb1bd
In other words, the driver needs to get the physical address of the frame
buffer (mapped via iommu) in order to set some DRM-specific register.
Yeah, the above code is somewhat hackish. I would love to replace
this part by a more standard approach.
Thanks,
Mauro
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: hikey9xx: Kconfig: add regulator dependency
The regulator driver needs it, as otherwise it will produce errors when creating vmlinux. Reported-by: Greg Kroah-Hartman Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/hikey9xx/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/hikey9xx/Kconfig b/drivers/staging/hikey9xx/Kconfig index a004839e8fa9..0e97b5b9a56a 100644 --- a/drivers/staging/hikey9xx/Kconfig +++ b/drivers/staging/hikey9xx/Kconfig @@ -31,6 +31,7 @@ config MFD_HI6421_SPMI config REGULATOR_HI6421V600 tristate "HiSilicon Hi6421v600 PMIC voltage regulator support" depends on MFD_HI6421_SPMI && OF + depends on REGULATOR help This driver provides support for the voltage regulators on HiSilicon Hi6421v600 PMU / Codec IC. -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] staging: hikey9xx: fix Kconfig dependency chain
Em Tue, 18 Aug 2020 17:07:04 +0200 Greg Kroah-Hartman escreveu: > On Tue, Aug 18, 2020 at 04:58:53PM +0200, Mauro Carvalho Chehab wrote: > > Both the SPMI controller and the SPMI PMIC driver > > depends on the SPMI bus support. > > > > The dependency for the regulator is also wrong: > > it should depends on the SPMI version of the HiSilicon 6421, > > and not on the normal one. > > > > Signed-off-by: Mauro Carvalho Chehab > > --- > > drivers/staging/hikey9xx/Kconfig | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/staging/hikey9xx/Kconfig > > b/drivers/staging/hikey9xx/Kconfig > > index 76267b9be562..a004839e8fa9 100644 > > --- a/drivers/staging/hikey9xx/Kconfig > > +++ b/drivers/staging/hikey9xx/Kconfig > > @@ -5,6 +5,7 @@ config SPMI_HISI3670 > > tristate "Hisilicon 3670 SPMI Controller" > > select IRQ_DOMAIN_HIERARCHY > > depends on HAS_IOMEM > > + depends on SPMI > > help > > If you say yes to this option, support will be included for the > > built-in SPMI PMIC Arbiter interface on Hisilicon 3670 > > @@ -14,6 +15,7 @@ config SPMI_HISI3670 > > config MFD_HI6421_SPMI > > tristate "HiSilicon Hi6421v600 SPMI PMU/Codec IC" > > depends on OF > > + depends on SPMI > > select MFD_CORE > > help > > Add support for HiSilicon Hi6421v600 SPMI PMIC. Hi6421 includes > > @@ -28,7 +30,7 @@ config MFD_HI6421_SPMI > > # to be placed at drivers/regulator > > config REGULATOR_HI6421V600 > > tristate "HiSilicon Hi6421v600 PMIC voltage regulator support" > > - depends on MFD_HI6421_PMIC && OF > > + depends on MFD_HI6421_SPMI && OF > > help > > This driver provides support for the voltage regulators on > > HiSilicon Hi6421v600 PMU / Codec IC. > > Better, but now I get the following build error: > > ERROR: modpost: "regulator_map_voltage_iterate" > [drivers/staging/hikey9xx/hi6421v600-regulator.ko] undefined! > ERROR: modpost: "regulator_list_voltage_table" > [drivers/staging/hikey9xx/hi6421v600-regulator.ko] undefined! > ERROR: modpost: "of_get_regulator_init_data" > [drivers/staging/hikey9xx/hi6421v600-regulator.ko] undefined! > ERROR: modpost: "regulator_register" > [drivers/staging/hikey9xx/hi6421v600-regulator.ko] undefined! > ERROR: modpost: "regulator_unregister" > [drivers/staging/hikey9xx/hi6421v600-regulator.ko] undefined! > ERROR: modpost: "rdev_get_drvdata" > [drivers/staging/hikey9xx/hi6421v600-regulator.ko] undefined! > > Someone need CONFIG_REGULATOR enabled? Yep. Sorry for that! > > Another follow-on patch? :) :) Just to make sure, I did a clean compilation here, starting from nomodconfig, and adding just CONFIG_ARCH* and the dependencies needed by those symbols. It build fine here. So, I guess it should be ok this time (famous last words...) Thanks, Mauro ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
On 2020-08-18 16:29, Mauro Carvalho Chehab wrote:
Hi Robin,
Em Tue, 18 Aug 2020 15:47:55 +0100
Robin Murphy escreveu:
On 2020-08-17 08:49, Mauro Carvalho Chehab wrote:
Add a driver for the Kirin 960/970 iommu.
As on the past series, this starts from the original 4.9 driver from
the 96boards tree:
https://github.com/96boards-hikey/linux/tree/hikey970-v4.9
The remaining patches add SPDX headers and make it build and run with
the upstream Kernel.
Chenfeng (1):
iommu: add support for HiSilicon Kirin 960/970 iommu
Mauro Carvalho Chehab (15):
iommu: hisilicon: remove default iommu_map_sg handler
iommu: hisilicon: map and unmap ops gained new arguments
iommu: hisi_smmu_lpae: rebase it to work with upstream
iommu: hisi_smmu: remove linux/hisi/hisi-iommu.h
iommu: hisilicon: cleanup its code style
iommu: hisi_smmu_lpae: get rid of IOMMU_SEC and IOMMU_DEVICE
iommu: get rid of map/unmap tile functions
iommu: hisi_smmu_lpae: use the right code to get domain-priv data
iommu: hisi_smmu_lpae: convert it to probe_device
iommu: add Hisilicon Kirin970 iommu at the building system
iommu: hisi_smmu_lpae: cleanup printk macros
iommu: hisi_smmu_lpae: make OF compatible more standard
Echoing the other comments about none of the driver patches being CC'd
to the IOMMU list...
Still, I dug the series up on lore and frankly I'm not sure what to make
of it - AFAICS the "driver" is just yet another implementation of Arm
LPAE pagetable code, with no obvious indication of how those pagetables
ever get handed off to IOMMU hardware (and indeed no indication of IOMMU
hardware at all). Can you explain how it's supposed to work?
And as a pre-emptive strike, we really don't need any more LPAE
implementations - that's what the io-pgtable library is all about (which
incidentally has been around since 4.0...). I think that should make the
issue of preserving authorship largely moot since there's no need to
preserve most of the code anyway ;)
I didn't know about that, since I got a Hikey 970 board for the first time
about one month ago, and that's the first time I looked into iommu code.
My end goal with this is to make the DRM/KMS driver to work with upstream
Kernels.
The full patch series are at:
https://github.com/mchehab/linux/commits/hikey970/to_upstream-2.0-v1.1
(I need to put a new version there, after some changes due to recent
upstream discussions at the regulator's part of the code)
Basically, the DT binding has this, for IOMMU:
smmu_lpae {
compatible = "hisilicon,smmu-lpae";
};
...
dpe: dpe@e860 {
compatible = "hisilicon,kirin970-dpe";
memory-region = <&drm_dma_reserved>;
...
iommu_info {
start-addr = <0x8000>;
size = <0xbfff8000>;
};
}
This is used by kirin9xx_drm_dss.c in order to enable and use
the iommu:
static int dss_enable_iommu(struct platform_device *pdev, struct
dss_hw_ctx *ctx)
{
struct device *dev = NULL;
dev = &pdev->dev;
/* create iommu domain */
ctx->mmu_domain = iommu_domain_alloc(dev->bus);
if (!ctx->mmu_domain) {
pr_err("iommu_domain_alloc failed!\n");
return -EINVAL;
}
iommu_attach_device(ctx->mmu_domain, dev);
return 0;
}
The only place where the IOMMU domain is used is on this part of the
code(error part simplified here) [1]:
void hisi_dss_smmu_on(struct dss_hw_ctx *ctx)
{
uint64_t fama_phy_pgd_base;
uint32_t phy_pgd_base;
...
fama_phy_pgd_base = iommu_iova_to_phys(ctx->mmu_domain, 0);
phy_pgd_base = (uint32_t)fama_phy_pgd_base;
if (WARN_ON(!phy_pgd_base))
return;
set_reg(smmu_base + SMMU_CB_TTBR0, phy_pgd_base, 32, 0);
}
[1]
https://github.com/mchehab/linux/commit/36da105e719b47bbe9d6cb7e5619b30c7f3eb1bd
In other words, the driver needs to get the physical address of the frame
buffer (mapped via iommu) in order to set some DRM-specific register.
Yeah, the above code is somewhat hackish. I would love to replace
this part by a more standard approach.
OK, so from a quick look at that, my impression is that your display
controller has its own MMU and you don't need to pretend to use the
IOMMU API at all. Just have the DRM driver use io-pgtable directly to
run its own set of ARM_32_LPAE_S1 pagetables - see Panfrost for an
example (but try to ignore the wacky "Mali LPAE" format).
Robin.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: kpc2000: kpc_dma: fix spelling mistake "for for" -> "for"
From: Colin Ian King
There are a couple of duplicated "for" spelling mistakes in dev_err
error messages. Fix these.
Signed-off-by: Colin Ian King
---
drivers/staging/kpc2000/kpc_dma/fileops.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c
b/drivers/staging/kpc2000/kpc_dma/fileops.c
index dd716edd9b1b..e1c7c04f16fe 100644
--- a/drivers/staging/kpc2000/kpc_dma/fileops.c
+++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
@@ -53,7 +53,7 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
acd = kzalloc(sizeof(*acd), GFP_KERNEL);
if (!acd) {
- dev_err(&priv->ldev->pldev->dev, "Couldn't kmalloc space for
for the aio data\n");
+ dev_err(&priv->ldev->pldev->dev, "Couldn't kmalloc space for
the aio data\n");
return -ENOMEM;
}
memset(acd, 0x66, sizeof(struct aio_cb_data));
@@ -69,7 +69,7 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
acd->user_pages = kcalloc(acd->page_count, sizeof(struct page *),
GFP_KERNEL);
if (!acd->user_pages) {
- dev_err(&priv->ldev->pldev->dev, "Couldn't kmalloc space for
for the page pointers\n");
+ dev_err(&priv->ldev->pldev->dev, "Couldn't kmalloc space for
the page pointers\n");
rv = -ENOMEM;
goto err_alloc_userpages;
}
--
2.27.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: kpc2000: kpc_dma: fix spelling mistake "for for" -> "for"
On Tue, 2020-08-18 at 17:46 +0100, Colin King wrote:
> There are a couple of duplicated "for" spelling mistakes in dev_err
> error messages. Fix these.
Might as well remove the messages instead.
> diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c
> b/drivers/staging/kpc2000/kpc_dma/fileops.c
[]
> @@ -53,7 +53,7 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
>
> acd = kzalloc(sizeof(*acd), GFP_KERNEL);
> if (!acd) {
> - dev_err(&priv->ldev->pldev->dev, "Couldn't kmalloc space for
> for the aio data\n");
> + dev_err(&priv->ldev->pldev->dev, "Couldn't kmalloc space for
> the aio data\n");
> return -ENOMEM;
> }
> memset(acd, 0x66, sizeof(struct aio_cb_data));
Also there's no reason to use kzalloc then a memset over
the alloc'ed memory or a different sizeof style for the
alloc then memset.
Setting the struct content to 0x66 does seem odd.
> @@ -69,7 +69,7 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
> acd->user_pages = kcalloc(acd->page_count, sizeof(struct page *),
> GFP_KERNEL);
> if (!acd->user_pages) {
> - dev_err(&priv->ldev->pldev->dev, "Couldn't kmalloc space for
> for the page pointers\n");
> + dev_err(&priv->ldev->pldev->dev, "Couldn't kmalloc space for
> the page pointers\n");
> rv = -ENOMEM;
> goto err_alloc_userpages;
> }
Maybe:
---
drivers/staging/kpc2000/kpc_dma/fileops.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c
b/drivers/staging/kpc2000/kpc_dma/fileops.c
index dd716edd9b1b..c5330ad6b175 100644
--- a/drivers/staging/kpc2000/kpc_dma/fileops.c
+++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
@@ -51,12 +51,11 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
ldev = priv->ldev;
- acd = kzalloc(sizeof(*acd), GFP_KERNEL);
- if (!acd) {
- dev_err(&priv->ldev->pldev->dev, "Couldn't kmalloc space for
for the aio data\n");
+ acd = kmalloc(sizeof(*acd), GFP_KERNEL);
+ if (!acd)
return -ENOMEM;
- }
- memset(acd, 0x66, sizeof(struct aio_cb_data));
+
+ memset(acd, 0x66, sizeof(*acd));
acd->priv = priv;
acd->ldev = priv->ldev;
@@ -69,7 +68,6 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
acd->user_pages = kcalloc(acd->page_count, sizeof(struct page *),
GFP_KERNEL);
if (!acd->user_pages) {
- dev_err(&priv->ldev->pldev->dev, "Couldn't kmalloc space for
for the page pointers\n");
rv = -ENOMEM;
goto err_alloc_userpages;
}
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] staging: hikey9xx: fix Kconfig dependency chain
On Tue, Aug 18, 2020 at 06:07:16PM +0200, Mauro Carvalho Chehab wrote: > Em Tue, 18 Aug 2020 17:07:04 +0200 > Greg Kroah-Hartman escreveu: > > > On Tue, Aug 18, 2020 at 04:58:53PM +0200, Mauro Carvalho Chehab wrote: > > > Both the SPMI controller and the SPMI PMIC driver > > > depends on the SPMI bus support. > > > > > > The dependency for the regulator is also wrong: > > > it should depends on the SPMI version of the HiSilicon 6421, > > > and not on the normal one. > > > > > > Signed-off-by: Mauro Carvalho Chehab > > > --- > > > drivers/staging/hikey9xx/Kconfig | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/staging/hikey9xx/Kconfig > > > b/drivers/staging/hikey9xx/Kconfig > > > index 76267b9be562..a004839e8fa9 100644 > > > --- a/drivers/staging/hikey9xx/Kconfig > > > +++ b/drivers/staging/hikey9xx/Kconfig > > > @@ -5,6 +5,7 @@ config SPMI_HISI3670 > > > tristate "Hisilicon 3670 SPMI Controller" > > > select IRQ_DOMAIN_HIERARCHY > > > depends on HAS_IOMEM > > > + depends on SPMI > > > help > > > If you say yes to this option, support will be included for the > > > built-in SPMI PMIC Arbiter interface on Hisilicon 3670 > > > @@ -14,6 +15,7 @@ config SPMI_HISI3670 > > > config MFD_HI6421_SPMI > > > tristate "HiSilicon Hi6421v600 SPMI PMU/Codec IC" > > > depends on OF > > > + depends on SPMI > > > select MFD_CORE > > > help > > > Add support for HiSilicon Hi6421v600 SPMI PMIC. Hi6421 includes > > > @@ -28,7 +30,7 @@ config MFD_HI6421_SPMI > > > # to be placed at drivers/regulator > > > config REGULATOR_HI6421V600 > > > tristate "HiSilicon Hi6421v600 PMIC voltage regulator support" > > > - depends on MFD_HI6421_PMIC && OF > > > + depends on MFD_HI6421_SPMI && OF > > > help > > > This driver provides support for the voltage regulators on > > > HiSilicon Hi6421v600 PMU / Codec IC. > > > > Better, but now I get the following build error: > > > > ERROR: modpost: "regulator_map_voltage_iterate" > > [drivers/staging/hikey9xx/hi6421v600-regulator.ko] undefined! > > ERROR: modpost: "regulator_list_voltage_table" > > [drivers/staging/hikey9xx/hi6421v600-regulator.ko] undefined! > > ERROR: modpost: "of_get_regulator_init_data" > > [drivers/staging/hikey9xx/hi6421v600-regulator.ko] undefined! > > ERROR: modpost: "regulator_register" > > [drivers/staging/hikey9xx/hi6421v600-regulator.ko] undefined! > > ERROR: modpost: "regulator_unregister" > > [drivers/staging/hikey9xx/hi6421v600-regulator.ko] undefined! > > ERROR: modpost: "rdev_get_drvdata" > > [drivers/staging/hikey9xx/hi6421v600-regulator.ko] undefined! > > > > Someone need CONFIG_REGULATOR enabled? > > Yep. Sorry for that! > > > > Another follow-on patch? :) > > :) > > Just to make sure, I did a clean compilation here, starting from > nomodconfig, and adding just CONFIG_ARCH* and the dependencies > needed by those symbols. It build fine here. So, I guess it > should be ok this time (famous last words...) Let's see what falls out in linux-next, that's the best test :) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] block: convert tasklets to use new tasklet_setup() API
On Mon, 2020-08-17 at 13:02 -0700, Jens Axboe wrote: > On 8/17/20 12:48 PM, Kees Cook wrote: > > On Mon, Aug 17, 2020 at 12:44:34PM -0700, Jens Axboe wrote: > > > On 8/17/20 12:29 PM, Kees Cook wrote: > > > > On Mon, Aug 17, 2020 at 06:56:47AM -0700, Jens Axboe wrote: > > > > > On 8/17/20 2:15 AM, Allen Pais wrote: > > > > > > From: Allen Pais > > > > > > > > > > > > In preparation for unconditionally passing the > > > > > > struct tasklet_struct pointer to all tasklet > > > > > > callbacks, switch to using the new tasklet_setup() > > > > > > and from_tasklet() to pass the tasklet pointer explicitly. > > > > > > > > > > Who came up with the idea to add a macro 'from_tasklet' that > > > > > is just container_of? container_of in the code would be > > > > > _much_ more readable, and not leave anyone guessing wtf > > > > > from_tasklet is doing. > > > > > > > > > > I'd fix that up now before everything else goes in... > > > > > > > > As I mentioned in the other thread, I think this makes things > > > > much more readable. It's the same thing that the timer_struct > > > > conversion did (added a container_of wrapper) to avoid the > > > > ever-repeating use of typeof(), long lines, etc. > > > > > > But then it should use a generic name, instead of each sub-system > > > using some random name that makes people look up exactly what it > > > does. I'm not huge fan of the container_of() redundancy, but > > > adding private variants of this doesn't seem like the best way > > > forward. Let's have a generic helper that does this, and use it > > > everywhere. > > > > I'm open to suggestions, but as things stand, these kinds of > > treewide > > On naming? Implementation is just as it stands, from_tasklet() is > totally generic which is why I objected to it. from_member()? Not > great with naming... But I can see this going further and then we'll > suddenly have tons of these. It's not good for readability. Since both threads seem to have petered out, let me suggest in kernel.h: #define cast_out(ptr, container, member) \ container_of(ptr, typeof(*container), member) It does what you want, the argument order is the same as container_of with the only difference being you name the containing structure instead of having to specify its type. James ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] block: convert tasklets to use new tasklet_setup() API
On Tue, Aug 18, 2020 at 01:00:33PM -0700, James Bottomley wrote: > On Mon, 2020-08-17 at 13:02 -0700, Jens Axboe wrote: > > On 8/17/20 12:48 PM, Kees Cook wrote: > > > On Mon, Aug 17, 2020 at 12:44:34PM -0700, Jens Axboe wrote: > > > > On 8/17/20 12:29 PM, Kees Cook wrote: > > > > > On Mon, Aug 17, 2020 at 06:56:47AM -0700, Jens Axboe wrote: > > > > > > On 8/17/20 2:15 AM, Allen Pais wrote: > > > > > > > From: Allen Pais > > > > > > > > > > > > > > In preparation for unconditionally passing the > > > > > > > struct tasklet_struct pointer to all tasklet > > > > > > > callbacks, switch to using the new tasklet_setup() > > > > > > > and from_tasklet() to pass the tasklet pointer explicitly. > > > > > > > > > > > > Who came up with the idea to add a macro 'from_tasklet' that > > > > > > is just container_of? container_of in the code would be > > > > > > _much_ more readable, and not leave anyone guessing wtf > > > > > > from_tasklet is doing. > > > > > > > > > > > > I'd fix that up now before everything else goes in... > > > > > > > > > > As I mentioned in the other thread, I think this makes things > > > > > much more readable. It's the same thing that the timer_struct > > > > > conversion did (added a container_of wrapper) to avoid the > > > > > ever-repeating use of typeof(), long lines, etc. > > > > > > > > But then it should use a generic name, instead of each sub-system > > > > using some random name that makes people look up exactly what it > > > > does. I'm not huge fan of the container_of() redundancy, but > > > > adding private variants of this doesn't seem like the best way > > > > forward. Let's have a generic helper that does this, and use it > > > > everywhere. > > > > > > I'm open to suggestions, but as things stand, these kinds of > > > treewide > > > > On naming? Implementation is just as it stands, from_tasklet() is > > totally generic which is why I objected to it. from_member()? Not > > great with naming... But I can see this going further and then we'll > > suddenly have tons of these. It's not good for readability. > > Since both threads seem to have petered out, let me suggest in > kernel.h: > > #define cast_out(ptr, container, member) \ > container_of(ptr, typeof(*container), member) > > It does what you want, the argument order is the same as container_of > with the only difference being you name the containing structure > instead of having to specify its type. I like this! Shall I send this to Linus to see if this can land in -rc2 for use going forward? -- Kees Cook ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] block: convert tasklets to use new tasklet_setup() API
On Tue, 2020-08-18 at 13:10 -0700, Kees Cook wrote: > On Tue, Aug 18, 2020 at 01:00:33PM -0700, James Bottomley wrote: > > On Mon, 2020-08-17 at 13:02 -0700, Jens Axboe wrote: > > > On 8/17/20 12:48 PM, Kees Cook wrote: > > > > On Mon, Aug 17, 2020 at 12:44:34PM -0700, Jens Axboe wrote: > > > > > On 8/17/20 12:29 PM, Kees Cook wrote: > > > > > > On Mon, Aug 17, 2020 at 06:56:47AM -0700, Jens Axboe wrote: > > > > > > > On 8/17/20 2:15 AM, Allen Pais wrote: > > > > > > > > From: Allen Pais > > > > > > > > > > > > > > > > In preparation for unconditionally passing the > > > > > > > > struct tasklet_struct pointer to all tasklet > > > > > > > > callbacks, switch to using the new tasklet_setup() > > > > > > > > and from_tasklet() to pass the tasklet pointer > > > > > > > > explicitly. > > > > > > > > > > > > > > Who came up with the idea to add a macro 'from_tasklet' > > > > > > > that > > > > > > > is just container_of? container_of in the code would be > > > > > > > _much_ more readable, and not leave anyone guessing wtf > > > > > > > from_tasklet is doing. > > > > > > > > > > > > > > I'd fix that up now before everything else goes in... > > > > > > > > > > > > As I mentioned in the other thread, I think this makes > > > > > > things > > > > > > much more readable. It's the same thing that the > > > > > > timer_struct > > > > > > conversion did (added a container_of wrapper) to avoid the > > > > > > ever-repeating use of typeof(), long lines, etc. > > > > > > > > > > But then it should use a generic name, instead of each sub- > > > > > system > > > > > using some random name that makes people look up exactly what > > > > > it > > > > > does. I'm not huge fan of the container_of() redundancy, but > > > > > adding private variants of this doesn't seem like the best > > > > > way > > > > > forward. Let's have a generic helper that does this, and use > > > > > it > > > > > everywhere. > > > > > > > > I'm open to suggestions, but as things stand, these kinds of > > > > treewide > > > > > > On naming? Implementation is just as it stands, from_tasklet() is > > > totally generic which is why I objected to it. from_member()? Not > > > great with naming... But I can see this going further and then > > > we'll > > > suddenly have tons of these. It's not good for readability. > > > > Since both threads seem to have petered out, let me suggest in > > kernel.h: > > > > #define cast_out(ptr, container, member) \ > > container_of(ptr, typeof(*container), member) > > > > It does what you want, the argument order is the same as > > container_of with the only difference being you name the containing > > structure instead of having to specify its type. > > I like this! Shall I send this to Linus to see if this can land in > -rc2 for use going forward? Sure ... he's probably been lurking on this thread anyway ... it's about time he got off his arse^Wthe fence and made an executive decision ... James ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
On Tue, Aug 18, 2020 at 9:26 AM Robin Murphy wrote:
> On 2020-08-18 16:29, Mauro Carvalho Chehab wrote:
> > Em Tue, 18 Aug 2020 15:47:55 +0100
> > Basically, the DT binding has this, for IOMMU:
> >
> >
> > smmu_lpae {
> > compatible = "hisilicon,smmu-lpae";
> > };
> >
> > ...
> > dpe: dpe@e860 {
> > compatible = "hisilicon,kirin970-dpe";
> > memory-region = <&drm_dma_reserved>;
> > ...
> > iommu_info {
> > start-addr = <0x8000>;
> > size = <0xbfff8000>;
> > };
> > }
> >
> > This is used by kirin9xx_drm_dss.c in order to enable and use
> > the iommu:
> >
> >
> > static int dss_enable_iommu(struct platform_device *pdev, struct
> > dss_hw_ctx *ctx)
> > {
> > struct device *dev = NULL;
> >
> > dev = &pdev->dev;
> >
> > /* create iommu domain */
> > ctx->mmu_domain = iommu_domain_alloc(dev->bus);
> > if (!ctx->mmu_domain) {
> > pr_err("iommu_domain_alloc failed!\n");
> > return -EINVAL;
> > }
> >
> > iommu_attach_device(ctx->mmu_domain, dev);
> >
> > return 0;
> > }
> >
> > The only place where the IOMMU domain is used is on this part of the
> > code(error part simplified here) [1]:
> >
> > void hisi_dss_smmu_on(struct dss_hw_ctx *ctx)
> > {
> > uint64_t fama_phy_pgd_base;
> > uint32_t phy_pgd_base;
> > ...
> > fama_phy_pgd_base = iommu_iova_to_phys(ctx->mmu_domain, 0);
> > phy_pgd_base = (uint32_t)fama_phy_pgd_base;
> > if (WARN_ON(!phy_pgd_base))
> > return;
> >
> > set_reg(smmu_base + SMMU_CB_TTBR0, phy_pgd_base, 32, 0);
> > }
> >
> > [1]
> > https://github.com/mchehab/linux/commit/36da105e719b47bbe9d6cb7e5619b30c7f3eb1bd
> >
> > In other words, the driver needs to get the physical address of the frame
> > buffer (mapped via iommu) in order to set some DRM-specific register.
> >
> > Yeah, the above code is somewhat hackish. I would love to replace
> > this part by a more standard approach.
>
> OK, so from a quick look at that, my impression is that your display
> controller has its own MMU and you don't need to pretend to use the
> IOMMU API at all. Just have the DRM driver use io-pgtable directly to
> run its own set of ARM_32_LPAE_S1 pagetables - see Panfrost for an
> example (but try to ignore the wacky "Mali LPAE" format).
Yea. For the HiKey960, there was originally a similar patch series but
it was refactored out and the (still out of tree) DRM driver I'm
carrying doesn't seem to need it (though looking we still have the
iommu_info subnode in the dts that maybe needs to be cleaned up).
thanks
-john
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[staging:staging-linus] BUILD SUCCESS 1dffeb8b8b4c261c45416d53c75ea51e6ece1770
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git staging-linus branch HEAD: 1dffeb8b8b4c261c45416d53c75ea51e6ece1770 staging: greybus: audio: fix uninitialized value issue elapsed time: 725m configs tested: 72 configs skipped: 1 The following configs have been built successfully. More configs may be tested in the coming days. arm defconfig arm64allyesconfig arm64 defconfig arm allyesconfig arm allmodconfig ia64 allmodconfig ia64defconfig ia64 allyesconfig m68k allmodconfig m68kdefconfig m68k allyesconfig nios2 defconfig arc allyesconfig nds32 allnoconfig c6x allyesconfig nds32 defconfig nios2allyesconfig cskydefconfig alpha defconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig arc defconfig sh allmodconfig parisc defconfig s390 allyesconfig parisc allyesconfig s390defconfig i386 allyesconfig sparcallyesconfig sparc defconfig i386defconfig mips allyesconfig mips allmodconfig powerpc defconfig powerpc allyesconfig powerpc allmodconfig powerpc allnoconfig i386 randconfig-a005-20200818 i386 randconfig-a002-20200818 i386 randconfig-a001-20200818 i386 randconfig-a006-20200818 i386 randconfig-a003-20200818 i386 randconfig-a004-20200818 x86_64 randconfig-a013-20200818 x86_64 randconfig-a016-20200818 x86_64 randconfig-a012-20200818 x86_64 randconfig-a011-20200818 x86_64 randconfig-a014-20200818 x86_64 randconfig-a015-20200818 i386 randconfig-a016-20200818 i386 randconfig-a011-20200818 i386 randconfig-a015-20200818 i386 randconfig-a013-20200818 i386 randconfig-a012-20200818 i386 randconfig-a014-20200818 x86_64 randconfig-a006-20200819 x86_64 randconfig-a001-20200819 x86_64 randconfig-a003-20200819 x86_64 randconfig-a005-20200819 x86_64 randconfig-a004-20200819 x86_64 randconfig-a002-20200819 riscvallyesconfig riscv allnoconfig riscv defconfig riscvallmodconfig x86_64 rhel x86_64 allyesconfig x86_64rhel-7.6-kselftests x86_64 defconfig x86_64 rhel-8.3 x86_64 kexec --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[driver-core:readfile] BUILD SUCCESS d4b51a26f12c944f70a685fbadca460e24b8cdd6
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git readfile branch HEAD: d4b51a26f12c944f70a685fbadca460e24b8cdd6 readfile.2: new page describing readfile(2) elapsed time: 721m configs tested: 66 configs skipped: 1 The following configs have been built successfully. More configs may be tested in the coming days. arm defconfig arm64allyesconfig arm64 defconfig arm allyesconfig arm allmodconfig ia64 allmodconfig ia64defconfig ia64 allyesconfig m68k allmodconfig m68kdefconfig m68k allyesconfig nios2 defconfig arc allyesconfig nds32 allnoconfig c6x allyesconfig nds32 defconfig nios2allyesconfig cskydefconfig alpha defconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig arc defconfig sh allmodconfig parisc defconfig s390 allyesconfig parisc allyesconfig s390defconfig i386 allyesconfig sparcallyesconfig sparc defconfig i386defconfig mips allyesconfig mips allmodconfig powerpc defconfig powerpc allyesconfig powerpc allmodconfig powerpc allnoconfig i386 randconfig-a005-20200818 i386 randconfig-a002-20200818 i386 randconfig-a001-20200818 i386 randconfig-a006-20200818 i386 randconfig-a003-20200818 i386 randconfig-a004-20200818 x86_64 randconfig-a013-20200818 x86_64 randconfig-a016-20200818 x86_64 randconfig-a012-20200818 x86_64 randconfig-a011-20200818 x86_64 randconfig-a014-20200818 x86_64 randconfig-a015-20200818 i386 randconfig-a016-20200818 i386 randconfig-a011-20200818 i386 randconfig-a015-20200818 i386 randconfig-a013-20200818 i386 randconfig-a012-20200818 i386 randconfig-a014-20200818 riscvallyesconfig riscv allnoconfig riscv defconfig riscvallmodconfig x86_64 rhel x86_64 allyesconfig x86_64rhel-7.6-kselftests x86_64 defconfig x86_64 rhel-8.3 x86_64 kexec --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[driver-core:debugfs_cleanup] BUILD SUCCESS f7dc69c6674f761485cbd5574c35a7769a1ad31d
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git debugfs_cleanup branch HEAD: f7dc69c6674f761485cbd5574c35a7769a1ad31d debugfs: remove return value of debugfs_create_devm_seqfile() elapsed time: 721m configs tested: 66 configs skipped: 1 The following configs have been built successfully. More configs may be tested in the coming days. arm defconfig arm64allyesconfig arm64 defconfig arm allyesconfig arm allmodconfig ia64 allmodconfig ia64defconfig ia64 allyesconfig m68k allmodconfig m68kdefconfig m68k allyesconfig nios2 defconfig arc allyesconfig nds32 allnoconfig c6x allyesconfig nds32 defconfig nios2allyesconfig cskydefconfig alpha defconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig arc defconfig sh allmodconfig parisc defconfig s390 allyesconfig parisc allyesconfig s390defconfig i386 allyesconfig sparcallyesconfig sparc defconfig i386defconfig mips allyesconfig mips allmodconfig powerpc defconfig powerpc allyesconfig powerpc allmodconfig powerpc allnoconfig i386 randconfig-a005-20200818 i386 randconfig-a002-20200818 i386 randconfig-a001-20200818 i386 randconfig-a006-20200818 i386 randconfig-a003-20200818 i386 randconfig-a004-20200818 x86_64 randconfig-a013-20200818 x86_64 randconfig-a016-20200818 x86_64 randconfig-a012-20200818 x86_64 randconfig-a011-20200818 x86_64 randconfig-a014-20200818 x86_64 randconfig-a015-20200818 i386 randconfig-a016-20200818 i386 randconfig-a011-20200818 i386 randconfig-a015-20200818 i386 randconfig-a013-20200818 i386 randconfig-a012-20200818 i386 randconfig-a014-20200818 riscvallyesconfig riscv allnoconfig riscv defconfig riscvallmodconfig x86_64 rhel x86_64 allyesconfig x86_64rhel-7.6-kselftests x86_64 defconfig x86_64 rhel-8.3 x86_64 kexec --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
