Hi Michael,
On Tue, Mar 6, 2018 at 2:33 AM, Michael Schmitz <schmitzmic@xxxxxxxxx> wrote:
> On Tue, Mar 6, 2018 at 12:29 AM, Geert Uytterhoeven
> <geert@xxxxxxxxxxxxxx> wrote:
>>> +static unsigned char ctrl_data; /* Keep backup of the stuff written
>>> + * to ctrl_reg. Always write a copy
>>> + * to this register when writing to
>>> + * the hardware register!
>>> + */
>>
>> This should be part of the device's zorro_esp_priv.
>
> It's only used in the cyber_dma_setup, and I not actually read
> anywhere else. Might as well be on the stack instead of static...
OK, I missed that. Yes, local variable is better.
>>> +static int zorro_esp_init_one(struct zorro_dev *z,
>>> + const struct zorro_device_id *ent)
BTW, please call the probe/remove functions zorro_esp_probe() resp.
zorro_esp_remove().
>>> +{
>>> + struct scsi_host_template *tpnt = &scsi_esp_template;
>>> + struct Scsi_Host *host;
>>> + struct esp *esp;
>>> + struct zorro_driver_data *zdd;
>>> + struct zorro_esp_priv *zep;
>>> + unsigned long board, ioaddr, dmaaddr;
>>> + int err = -ENOMEM;
>>
>> Initialization not needed.
>
> The initialized err variable is used when bailing out ..
>
>>> +
>>> + board = zorro_resource_start(z);
>>> + zdd = (struct zorro_driver_data *)ent->driver_data;
>>> +
>>> + pr_info(PFX "%s found at address 0x%lx.\n", zdd->name, board);
>>> +
>>> + if (zdd->absolute) {
>>> + ioaddr = zdd->offset;
>>> + dmaaddr = zdd->dma_offset;
>>> + } else {
>>> + ioaddr = board + zdd->offset;
>>> + dmaaddr = board + zdd->dma_offset;
>>> + }
>>> +
>>> + if (!zorro_request_device(z, zdd->name)) {
>>> + pr_err(PFX "cannot reserve region 0x%lx, abort\n",
>>> + board);
>>> + return -EBUSY;
>>> + }
>>> +
>>> + /* Fill in the required pieces of hostdata */
>>> +
>>> + host = scsi_host_alloc(tpnt, sizeof(struct esp));
>>> +
>>> + if (!host) {
>>> + pr_err(PFX "No host detected; board configuration problem?\n");
>>> + goto out_free;
>>> + }
>
> here. But I can add the err=-NOMEM here.
After out_free it returns fixed -ENODEV ;-)
Doing "err = -ENOMEM" here, and returning err at the end is better, as
it propagates meaningful error codes.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
