Re: [PATCH v2] ata/pata_buddha: Probe via modalias instead of initcall

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/25/2019 07:03 PM, Bartlomiej Zolnierkiewicz wrote:
+err_ata_host_alloc:
+	switch (type) {
+	case BOARD_BUDDHA:
+	case BOARD_CATWEASEL:
+	default:
+		devm_release_mem_region(&z->dev,
+					board + BUDDHA_BASE1,
+					0x800);

Could you please explain why this is needed now?

[ The whole idea of using devm_* helpers is to not have to release
  resources explicitly. ]

My mistake. Thanks, I'll fix it.


+static void pata_buddha_remove(struct zorro_dev *zdev)
+{
+	/* NOT IMPLEMENTED */
+
+	WARN_ONCE(1, "pata_buddha: Attempted to remove driver. This is not implemented yet.\n");

Please try to implement it, should be as simple as:

static void pata_buddha_remove(struct zorro_dev *zdev)
{
	struct ata_host *host = dev_get_drvdata(&zdev->dev);

	ata_host_detach(host);
}

[ ata_host_alloc() in pata_buddha_probe() sets drvdata to host ]

Seeing as the driver is almost 1:1 the same as pata_gayle, I see no reason against this.

Do you need me to test module removal on the real machine, or is it okay to take your suggestion as-is, given that it is already accepted in pata_gayle?


The rest of the patch looks fine, thanks for working on this driver.

Thanks for reviewing it, and thanks for porting buddha to libata!


PS Next time please also use scripts/get_maintainer.pl script to get
   the list of people that should be added to Cc:, i.e.:

$ ./scripts/get_maintainer.pl -f drivers/ata/pata_buddha.c
Bartlomiej Zolnierkiewicz <b.zolnierkie@xxxxxxxxxxx> (maintainer:LIBATA PATA DRIVERS)
Jens Axboe <axboe@xxxxxxxxx> (maintainer:LIBATA PATA DRIVERS)
linux-ide@xxxxxxxxxxxxxxx (open list:LIBATA PATA DRIVERS)
linux-kernel@xxxxxxxxxxxxxxx (open list)

[ I've also added John, Michael & Geert to Cc: (as they were all
  involved in the development of the initial driver version). ]

Oops, thanks for fixing this.


Max



[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux