Hi James,
>> > +static irqreturn_t img_irq_handler(int irq, void *dev_id) {
>> > + struct img_hash_dev *hdev = dev_id;
>> > + u32 reg;
>> > +
>> > + reg = img_hash_read(hdev, CR_INTSTAT);
>> > + img_hash_write(hdev, CR_INTCLEAR, reg);
>> > +
>> > + if (reg & CR_INT_NEW_RESULTS_SET) {
>> > + dev_dbg(hdev->dev, "IRQ CR_INT_NEW_RESULTS_SET\n");
>> > + if (DRIVER_FLAGS_BUSY & hdev->flags) {
>> > + hdev->flags |= DRIVER_FLAGS_OUTPUT_READY;
>> > + if (!(DRIVER_FLAGS_CPU & hdev->flags))
>> > + hdev->flags |= DRIVER_FLAGS_DMA_READY;
>> > + tasklet_schedule(&hdev->done_task);
>>
>> Since this done_task only gets scheduled from here, why not make this a
>> threaded IRQ handler and just do the work here instead of separating it
>> between a hard IRQ handler and a tasklet?
>
> What is the advantage of doing that? i.e is this simple case worth setting up an extra thread?
I believe threaded IRQ handlers are now preferred to using a hard IRQ
handler + tasklet when possible, partly because people tend to screw
up tasklet usage.
>> > + err = PTR_ERR(hdev->io_base);
>> > + goto hash_io_err;
>> > + }
>> > +
>> > + /* Write port (DMA or CPU) */
>> > + hash_res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> > + if (!hash_res) {
>> > + dev_err(dev, "no write port resource info\n");
>> > + err = -ENODEV;
>> > + goto res_err;
>> > + }
>> > + hdev->bus_addr = hash_res->start;
>>
>> Maybe set this after devm_ioremap_resource() succeeds and avoid the extra
>> error check?
>
> Not quite following you here - which check are you saying can be removed?
You can remove the if (hash_res) check if you set hdev->bus_addr after
devm_ioremap_resource().
Thanks,
Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
