Am 07.03.2018 um 13:54 schrieb Finn Thain:
> On Wed, 7 Mar 2018, Michael Schmitz wrote:
>> The major obstacle now seems to be dynamic allocation of the driver
>> private data and storing a pointer to that in a way that it can be
>> retrieved using just the esp pointer. dev_set_drvdata(esp->dev, zep)
>> causes the module load to crash ...
> I've just noticed that most esp drivers do this:
> static int esp_foo_probe(struct platform_device *dev)
> esp->dev = dev;
> But the esp->dev->dev dereferencing sometimes gets overlooked, resulting
> in a pointer to a struct platform_device being used where a pointer to a
> struct device should be used (i.e. dma_*() calls). I will look into fixing
> this up. sun_esp.c doesn't have this problem, but the other drivers do.
> I don't think any of that applies to your zorro_esp code because the
> version you sent does this,
> esp->dev = &z->dev;
> which seems fine to me. But it could end up more convenient to use the
> sun_esp approach and set esp->dev = z.
It just adds another dereferencing step in the dma_map functions which
we only need to do once here.
But sun_esp also does
i.e. not only does it store the struct device pointer in the esp struct
(by indirection through the platform device struct), but also the struct
esp pointer in struct device.
> I suspect that the problem with zorro_esp is that sometimes you use the
> esp->dev->driver_data pointer as the struct Scsi_Host pointer, and at
> other times you use it for the struct zorro_driver_data pointer. (I think
> I see now why you put the esp pointer in struct zorro_driver_data.)
The zorro_esp_priv pointer (zorro_esp_driver_data pointer are only used
to store board specific config data needed for probe), but yes, you've
found my error.
I overlooked that dev_set_drvdata() and zorro_set_drvdata() will store
their payload in the same struct device instance. Using one for struct
zorro_esp_priv and the other for struct Scsi_host is just asking for
trouble. Thanks for jogging my memory ...
Since there is no other place for me to put driver private data, I need
to change the use of that field to point to the current zorro_esp_priv
instance, and retrieve the struct esp pointer from there. I can retrieve
the host pointer from struct esp so all should be well.
This is a little unusual so I better add a few comments to save the next
maintainer from unnecessary headache.
> If you like, email the current version to me or push it to a repo
> somewhere and I'll take a look at it.
I'll take you up on that offer another time, but with the use of
dev->driver_data fixed, the driver no longer crashes. I shouldn't
hack kernel code in a rush...
Now on to mangle the rest of the issues raised in the review...