Received: by 2002:a4f:b056:0:0:0:0:0 with SMTP id m22csp2661191ivi; Tue, 15 Sep 2020 16:00:42 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwXY8Pxy/1sAM/IdWWbMOKqcLVb4rRdS9qBmbLC6hPVVv2oIblKuVgh5Gt9Glr4bkX9f3yn X-Received: by 2002:a17:906:c447:: with SMTP id ck7mr22094947ejb.358.1600210841911; Tue, 15 Sep 2020 16:00:41 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1600210841; cv=none; d=google.com; s=arc-20160816; b=vLTtqJp3YXoubfAhH5GXzmivmfLosXgrRbhWkYZQDx8XTNMhuKh1heY/ZQshAjV5CC bUB3rEjnwpx0lTlUF07lFYz4qZl8Ji9SV/qxFfUr+geJ3fnn1W0ZwWNH6XeNhhYJ8pyd iKojOuyaxD7bb1fUjtU8alk08eUn5Osl/dqFdBR+L8+EmhWb8ldhpkQ4wUhsGd5dkyd8 EnuEdttjrcyHTfCB7Yc/fxEIibfS7u0wQuh3MFPnsmb6Y3aopN2CZzxWUHcOot5TxJ0V mcA7Y7xx4PsdvF48c534tIvo63dRQhtEpm5PpRkEduBS/MDaov41VMyvUhXT9njxD0gB DFpw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:mime-version:references:message-id :in-reply-to:subject:cc:to:from:date; bh=aDvmVQQw6umJKuacCBMWRefedbXV0P+qzn+NHs+1aOg=; b=PEcH5PBAp1ky7QQMG0QxODjxvGoCZbI8RBtikAzGK4+m80u1Q8ebLKbbqfiLEJOR2r MPE+WsKrciI7jDq4S9ucmvAwf+Inmq0cGhSgcu/w5EtgxTV4ZlvRuTiPkbRmagUvDVbN 8sXg6wmfVvsGaAv1KcrCBZS/soZFlQKCdQAubz+c/3H5zRR4KjuWC9jvGdu4tj0Bwu+G 6JkoBp9ck8vwCoPgQrVMPKBSqP76BXiu5PvDdYfFW5OKU6NY0PUt263b+vUCtKYuodXh qYWX5tCWleqljHwrHPmttGuhL7gzfnWFsgMTx9v8XPDorkMxb3ajLmcSLsZfAn6Mwn5I N8/Q== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id s22si10073336edt.207.2020.09.15.16.00.19; Tue, 15 Sep 2020 16:00:41 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727390AbgIOW52 (ORCPT + 99 others); Tue, 15 Sep 2020 18:57:28 -0400 Received: from kvm5.telegraphics.com.au ([98.124.60.144]:46572 "EHLO kvm5.telegraphics.com.au" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727419AbgIOW5E (ORCPT ); Tue, 15 Sep 2020 18:57:04 -0400 Received: from localhost (localhost.localdomain [127.0.0.1]) by kvm5.telegraphics.com.au (Postfix) with ESMTP id 9B1CF272F2; Tue, 15 Sep 2020 18:56:57 -0400 (EDT) Date: Wed, 16 Sep 2020 08:56:58 +1000 (AEST) From: Finn Thain To: Geert Uytterhoeven cc: "David S. Miller" , Bartlomiej Zolnierkiewicz , Joshua Thompson , linux-m68k , Linux Kernel Mailing List , linux-ide@vger.kernel.org Subject: Re: [PATCH v2] ide/macide: Convert Mac IDE driver to platform driver In-Reply-To: Message-ID: References: <3cf40b9df80a99a3eee6d3af79437016038f0a44.1600051331.git.fthain@telegraphics.com.au> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 15 Sep 2020, Geert Uytterhoeven wrote: > > > > --- a/drivers/ide/macide.c > > > > +++ b/drivers/ide/macide.c > > > > > > > @@ -109,42 +110,61 @@ static const char *mac_ide_name[] = > > > > * Probe for a Macintosh IDE interface > > > > */ > > > > > > > > -static int __init macide_init(void) > > > > +static int mac_ide_probe(struct platform_device *pdev) > > > > { > > > > > > > printk(KERN_INFO "ide: Macintosh %s IDE controller\n", > > > > mac_ide_name[macintosh_config->ide_type - 1]); > > > > > > > > - macide_setup_ports(&hw, base, irq); > > > > + macide_setup_ports(&hw, mem->start, irq->start); > > > > > > > > - return ide_host_add(&d, hws, 1, NULL); > > > > + rc = ide_host_add(&d, hws, 1, &host); > > > > + if (rc) > > > > + return rc; > > > > + > > > > + platform_set_drvdata(pdev, host); > > > > > > Move one up, to play it safe? > > > > > > > You mean, before calling ide_host_add? The 'host' pointer is uninitialized > > prior to that call. > > Oh right, so the IDE subsystem doesn't let you use the drvdata inside > your driver (besides in remove()) in a safe way :-( > The IDE subsystem does allow other patterns here. I could have changed ide_host_alloc() into ide_host_register() followed by ide_host_add() but I could not see any benefit from that change. A quick search for "platform_device" shows that the driver does not use any uninitialized driver_data pointer (because ide_ifr is a global). In your message of September 9th you readily reached the same conclusion when you reviewed v1. If mac_ide_probe() followed the usual pattern it might make review easier (as reviewers may not wish to consider the entire driver) but does that really make the code more "safe"?