lkml.org 
[lkml]   [2017]   [Jun]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 10/17] irqchip: New RISC-V PLIC Driver
On Wed, Jun 7, 2017 at 9:13 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> +struct plic_enable_context {
>> + atomic_t mask[32]; // 32-bit * 32-entry
>> +};

You use many '//' style comments in this file, please change them all to '/* */'
for consistency with kernel coding style.

>> +
>> +struct plic_priority {
>> + u32 prio[MAX_DEVICES];
>> +};
>> +
>> +struct plic_data {
>> + struct irq_chip chip;
>> + struct irq_domain *domain;
>> + u32 ndev;
>> + void __iomem *reg;
>> + int handlers;
>> + struct plic_handler *handler;
>> + char name[30];
>> +};
>> +
>> +struct plic_handler {
>> + struct plic_hart_context *context;
>> + struct plic_data *data;
>> +};
>> +
>> +static inline
>> +struct plic_hart_context *plic_hart_context(struct plic_data *data, size_t i)
>> +{
>> + return (struct plic_hart_context *)((char *)data->reg + HART_BASE + HART_SIZE*i);
>> +}

'data->reg' is an __iomem pointer, so when you build-test this with 'make C=1',
you should get a valid warning from sparse about an address space mismatch.
Please address all the warning from sparse.

>> +static void plic_disable(struct plic_data *data, int i, int hwirq)
>> +{
>> + struct plic_enable_context *enable = plic_enable_context(data, i);
>> +
>> + atomic_and(~(1 << (hwirq % 32)), &enable->mask[hwirq / 32]);
>> +}

In particular, you must not do atomic operations on MMIO pointers.
On most architectures these are explicitly disallowed and trap for
a good reason, as the hardware implementation behind atomics tend
to rely on the cache controller, while mmio registers are required
to be uncached.

>> + iowrite32(1, &priority->prio[d->hwirq]);

I would normally use 'readl' instead of 'iowrite32'. They may be the same
on riscv, but they have slightly different meaning in portable drivers.

Arnd

\
 
 \ /
  Last update: 2017-06-12 01:01    [W:0.131 / U:0.080 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site