From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 29E8CC433ED for ; Fri, 9 Apr 2021 04:00:19 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 85B7C610C8 for ; Fri, 9 Apr 2021 04:00:18 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 85B7C610C8 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=equinix.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:MIME-Version:Content-ID:In-Reply-To:References: Message-ID:Date:Subject:CC:To:From:Reply-To:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=gKgvuv8fZomdFSkhLOf8DGJygDO0OJ/vElpxDHjmbSg=; b=Y49/KzFLpktiNfoTM+lkkHoiV AkBfKYRSqumPYb+Gvamtbut/twPTKk1y7T0TAVUA6Bj0mIRB+/mepUwrbFWfQrW86hZM2EKxw75lX ZcNcFClMk8A5coacJmSFnThpLm3SUJeH4Tv1c6uHWa8y+HvyEt3TIaZ0zr/1sc0hWWJbYLdGmuTti d+86gDy3x+HforJoTdnRKV+M4kjw1X2t1U/F5TP72QemnZguzfAtU1jwUbH5uuUKb5VNGbsyl02zL X20k6zP6OTsgkW8Zux4nwQRX9cYnqj4ZbllYxXMmLB0sdt42rEA2didPrwAIMT1FyPPAzWEoLV1sI hYoauUYIQ==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lUiH1-009wYM-Bv; Fri, 09 Apr 2021 03:57:35 +0000 Received: from mx0b-00268f01.pphosted.com ([148.163.159.192]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lUiGu-009wX6-KX for linux-arm-kernel@lists.infradead.org; Fri, 09 Apr 2021 03:57:32 +0000 Received: from pps.filterd (m0165120.ppops.net [127.0.0.1]) by mx0b-00268f01.pphosted.com (8.16.0.43/8.16.0.43) with SMTP id 1393m8b2001076; Fri, 9 Apr 2021 03:57:14 GMT Received: from nam04-sn1-obe.outbound.protection.outlook.com (mail-sn1nam04lp2051.outbound.protection.outlook.com [104.47.44.51]) by mx0b-00268f01.pphosted.com with ESMTP id 37t83g18cv-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 09 Apr 2021 03:57:14 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=fysGYu7D9nmiMj91/k/MsCyR+OTY62C8wi2OP39n6E+EkLDrXD8GFV3hQrR+5QZxU8ef+PoYJaLDLOvB5aUDmNTN3T9yq5UGMV1GtceKscVGlwxOUIxMRgkzpWsYJqySL93/g/ENh5jUimtjTWypZ8IhY2OFT9tKgO0V8cy60TaukyUHDbBFQTpmUtxs+ufhibjHItdtm48zEmYzKZtFhCoplCamy5yOWbAnGwRIaOMHXElyOrdfW2A7H6cRwpzO2+3GU92p1e9TqN+ruRJ7oIQ6tYbG3yfiixrxsK2bkBbSOW0E05p8LRLlJWMN57yO/3tCmjn4luBqh0wwstl1SQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=bF4vdBsNOQphiRqkwoI7wN5jLlg0bEEC1WgwSwwHgtU=; b=AdcPbt1FAGvkTJBy9xQ+KNyQSm/I9q+sCNkCjZMrW506jKJxEouNnCC8cDkI3ECMVIHfUvxQ+nAfj3FDMBBQoPPAn55OdbdhVnOHt43/+uLuQ8FY1ih5MjSB5bA6X6bJWL3LzI3BaqXXJ8iCWyLIhIoKkt4QqlEUkYIOgBXkTizgUtM163RqjmbIxUEPw3pB9jeczMfpjkw33l8UM72QmtvNYO8cFDEaqKXHYXPrIfrxJunSa2pVHzLhghAfX9zu4d4h6HDEMocUC+fhpg7wvSChP9Bp2Xgl8A/9GMCX2Fbj+eEiwGykcF3pvlLMhQmkmVVuvn96y0XmGoLN/aTp+A== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=equinix.com; dmarc=pass action=none header.from=equinix.com; dkim=pass header.d=equinix.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=equinixinc.onmicrosoft.com; s=selector2-equinixinc-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=bF4vdBsNOQphiRqkwoI7wN5jLlg0bEEC1WgwSwwHgtU=; b=F0nFChJ+CzojOmPG6ecPW8tNkzik/H0AKuzEH/zFGrKLYIyRcc18niWsS3VG9TTIUTQcAUzruiSBMWBQFBvvHGSOnkGUHyqZjHtg0hB2uHGg7w8yY79Dz3MxexQVsH2rBqDzfSp5rcnkfFLw4vf5jIkb97COz46RXMlCNRO9JX4= Received: from DM5PR04MB0762.namprd04.prod.outlook.com (2603:10b6:3:f3::13) by DM6PR04MB6281.namprd04.prod.outlook.com (2603:10b6:5:1e8::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3999.28; Fri, 9 Apr 2021 03:57:12 +0000 Received: from DM5PR04MB0762.namprd04.prod.outlook.com ([fe80::4c98:aeb:87a8:13ad]) by DM5PR04MB0762.namprd04.prod.outlook.com ([fe80::4c98:aeb:87a8:13ad%5]) with mapi id 15.20.4020.017; Fri, 9 Apr 2021 03:57:12 +0000 From: Zev Weiss To: Andrew Jeffery CC: "openipmi-developer@lists.sourceforge.net" , "openbmc@lists.ozlabs.org" , "minyard@acm.org" , "devicetree@vger.kernel.org" , "ryan_chen@aspeedtech.com" , "tmaimon77@gmail.com" , "linux-aspeed@lists.ozlabs.org" , "avifishman70@gmail.com" , "venture@google.com" , "linus.walleij@linaro.org" , "linux-kernel@vger.kernel.org" , "tali.perry1@gmail.com" , "linux-gpio@vger.kernel.org" , "robh+dt@kernel.org" , "lee.jones@linaro.org" , "chiawei_wang@aspeedtech.com" , "linux-arm-kernel@lists.infradead.org" , "benjaminfair@google.com" Subject: Re: [PATCH v2 10/21] ipmi: kcs_bmc: Turn the driver data-structures inside-out Thread-Topic: [PATCH v2 10/21] ipmi: kcs_bmc: Turn the driver data-structures inside-out Thread-Index: AQHXLPRrPdVC3RFcvUCHtDapr0x+sg== Date: Fri, 9 Apr 2021 03:57:12 +0000 Message-ID: References: <20210319062752.145730-1-andrew@aj.id.au> <20210319062752.145730-10-andrew@aj.id.au> In-Reply-To: <20210319062752.145730-10-andrew@aj.id.au> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: aj.id.au; dkim=none (message not signed) header.d=none;aj.id.au; dmarc=none action=none header.from=equinix.com; x-originating-ip: [24.181.166.149] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 4db0e9de-949d-4d8e-79a4-08d8fb0b8e65 x-ms-traffictypediagnostic: DM6PR04MB6281: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:2150; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: KRWU46Wlp5ZC8rAArxvv+mJrUjw6mDKZnAGeAVUdZQUM4MbuCoyVl4MEGiU7DVQri9JnMEpJOSyiWVWDuKRxxc0cBCc7C6GwKo/W41qRRecqGIzlnWu5PcxWj3TBc6hRYbrD6K4MmeuRWmYVdflzezGVpcs/ZvUjobW6mrskLpIXByOdvFdSW7yG4JY73azqUlrJZpkDaPBG/SR8Umzc3/iqQeL+QFn142gDHESW777Jbsc/5LOSCjkby0iQGkeP3jbvpevFu7dTggAWHzGAh8qyHaG8qks5B20W2MchPFPhYrWTalXphj6jkWYtOB+KmYl1NAWDw56lokJNzXPe3PamTNmaSMCv6FLm1d/tOtkUQk0AolRMNbH0xBnQXJfuEhUtkwaLJO9VI5KWn1f07yICWOd6IFZR8nH84FEYZ9iT0C+yQANki1AIcIlWl9BdQRH0wxCa1H9j0uEPI9RIskRKm3ov11gU7rPX2rKeQd7jgojbnWliTjab+W3M1kXlk65ZsOfeauaT6FYvN3xIMSGHw1FIhenivVntVQNswAh50ECgCj4EYHv8DGcnx+3RJARpoTuaKX5+aReTPvTQ9JUaMAzQIiwrx+O+XiRUtgrTPNj7XSM9IY2gPFMtueY2WhN3QIVRTXGsmyWIiKyxjUwiobVh7Vxmb4PYfUbX8bY= x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DM5PR04MB0762.namprd04.prod.outlook.com; PTR:; CAT:NONE; SFS:(7916004)(4636009)(346002)(136003)(366004)(39860400002)(376002)(396003)(4326008)(8676002)(66476007)(66946007)(64756008)(26005)(6916009)(86362001)(7416002)(186003)(33716001)(2906002)(6512007)(8936002)(9686003)(38100700001)(76116006)(83380400001)(71200400001)(66446008)(66556008)(5660300002)(6506007)(6486002)(478600001)(316002)(54906003)(30864003); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata: =?us-ascii?Q?86jGy93V8im5Nbn4PGhZjVm7NbiCoWsx23eIylCEDH/rNL/AVxTvEMIsD+yt?= =?us-ascii?Q?J5f188UNOtXoo6ncBMmDG74Ee+zLm1OMTWdXjPK7Bqm+55jMMHws0xQzv9os?= =?us-ascii?Q?r/gtvjAgebI2g7zWVLBAIKwjiUte9Hk0cjzHAzqFehRUOEWsXIwOxcFJxUWS?= =?us-ascii?Q?QDoODR2LM6iZPgZ0a290UIylPtdYKyLbIc7NMUyrQBgi8QcfZn0A9EBdlgdv?= =?us-ascii?Q?VO3iZos2Zp7pGkumisykW5C+nOJ+YEa9VUnP5O5m+R2xYnGhGVFDWN1EuyYT?= =?us-ascii?Q?ovcO5nM3xjUxcUOypUwRA2W5rO7Hq1N2HtuMAd1aaXJXhhCx4YDaP80TZrBJ?= =?us-ascii?Q?KwF8L0oVTcbtm5V5RHBsz78VtW0EsrFPS7QyutTBOYG9HnMEZnUdCifaiXH8?= =?us-ascii?Q?z7hhVjNNqAbM8r5TV7AJJ05TUNgMV+rYLyQwATNMXdUfdQf3oq16Vk7exL6W?= =?us-ascii?Q?86ZU0b0Az8M1u+IkOYBthhlx8MoxC9y+4XKDqDpa07aeJPpiAo21HuAhZgs/?= =?us-ascii?Q?am05tal1Xy+70/gVLiS1TxljJFYn7CKYrWM5mgAB3LDPJggxNI11+da+HHpV?= =?us-ascii?Q?3qlsDooULm3PmOJP67RyWzx0e/+52dMh6Avtj8DbLe4K09THm0kjfdeXXNsY?= =?us-ascii?Q?701/YhcMsOUassk9u6TH4hiY6joSTQUwumRTtNsMhecG/vSjRLTy9Pta9ZxN?= =?us-ascii?Q?nUOGC58PnhF4NYWz9Qd7LbnMSmHBbFezF2KTU537EwP0/D2AQl1UsOGa8Y5S?= =?us-ascii?Q?H7zmQ9YQ5Cjp/Aihtf5vFU6BJVIzsTQ7FDlxJ83E5wXOTYW0oeEcv4ZlFlGE?= =?us-ascii?Q?j5hvwAQayT2N6u8hrF8DaYCSzxTgTiWkhJ89Nlgf3X+SmK3HAv2XJ/J+tJLX?= =?us-ascii?Q?GfZorL93KcBpWcL3nyLNL1izRhU7gQHbCt12gPU/fCmZPRkPYpBuo8cbjmDE?= =?us-ascii?Q?/Mr00iCIYoBaM+H+Vyq3Xnrg9zBcSzKrIcG6WpJ78msCbts6/XMTYdJzxZXg?= =?us-ascii?Q?SIVncsYBouF0viMF5q/Fc+0OB5ObNKS0ML3eQhcFinzRCjE44I16caZ/ZO3E?= =?us-ascii?Q?sw6UCVbQXS+JhPPtdBGcNDQ9zNQfURT7iVsYI9WX0WDH0FbL5XGJUfvbd/9+?= =?us-ascii?Q?BS4GR4W4+158gOzElbYE4rSwdYIruuAoS1PKtd60rdPQVu92x29V9ai/tnOo?= =?us-ascii?Q?SigEkdhsUvb7uFoAV9FdfT3C8sw8unqU3h87Y0X0jPa8zdWnWhtTpWcn4Li5?= =?us-ascii?Q?gqUmQ/FDQmRtbUSsMaINJXgEFgcMiS8Tzsw1fPXQVmEimqUii/FGcTcs4V2D?= =?us-ascii?Q?rGp9Xv4RwpKEuHYwfzloDMyO8Lo0L2GKsNW9keAVZnZmTA=3D=3D?= x-ms-exchange-transport-forked: True Content-ID: <17439EB88E9CC84BBF52D18548986FCE@namprd04.prod.outlook.com> MIME-Version: 1.0 X-OriginatorOrg: equinix.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM5PR04MB0762.namprd04.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 4db0e9de-949d-4d8e-79a4-08d8fb0b8e65 X-MS-Exchange-CrossTenant-originalarrivaltime: 09 Apr 2021 03:57:12.3213 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 72adb271-2fc7-4afe-a5ee-9de6a59f6bfb X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: Xjx4kOtZI4OzpvNzQUEsOAxn6wlcclUL9B/cLQowyU9tx3kEXOcGxC0/Ak8VcR6yBeYUSXiT5u7ds3qpsdWmSg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR04MB6281 X-Proofpoint-ORIG-GUID: JUEQpJoPCEqPaQMqpmz7kWtbMW6fmDEw X-Proofpoint-GUID: JUEQpJoPCEqPaQMqpmz7kWtbMW6fmDEw X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.391, 18.0.761 definitions=2021-04-09_03:2021-04-08, 2021-04-09 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 clxscore=1015 malwarescore=0 suspectscore=0 priorityscore=1501 mlxlogscore=999 adultscore=0 impostorscore=0 mlxscore=0 lowpriorityscore=0 spamscore=0 phishscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2104060000 definitions=main-2104090025 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210409_045729_729615_70C81A91 X-CRM114-Status: GOOD ( 18.62 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Mar 19, 2021 at 01:27:41AM CDT, Andrew Jeffery wrote: >Make the KCS device drivers responsible for allocating their own memory. > >Until now the private data for the device driver was allocated internal >to the private data for the chardev interface. This coupling required >the slightly awkward API of passing through the struct size for the >driver private data to the chardev constructor, and then retrieving a >pointer to the driver private data from the allocated chardev memory. > >In addition to being awkward, the arrangement prevents the >implementation of alternative userspace interfaces as the device driver >private data is not independent. > >Peel a layer off the onion and turn the data-structures inside out by >exploiting container_of() and embedding `struct kcs_device` in the >driver private data. > >Signed-off-by: Andrew Jeffery >--- > drivers/char/ipmi/kcs_bmc.c | 15 +++++-- > drivers/char/ipmi/kcs_bmc.h | 12 ++---- > drivers/char/ipmi/kcs_bmc_aspeed.c | 60 ++++++++++++++++----------- > drivers/char/ipmi/kcs_bmc_cdev_ipmi.c | 60 ++++++++++++++++++--------- > drivers/char/ipmi/kcs_bmc_npcm7xx.c | 37 ++++++++++------- > 5 files changed, 113 insertions(+), 71 deletions(-) > >diff --git a/drivers/char/ipmi/kcs_bmc.c b/drivers/char/ipmi/kcs_bmc.c >index ef5c48ffe74a..709b6bdec165 100644 >--- a/drivers/char/ipmi/kcs_bmc.c >+++ b/drivers/char/ipmi/kcs_bmc.c >@@ -44,12 +44,19 @@ int kcs_bmc_handle_event(struct kcs_bmc *kcs_bmc) > } > EXPORT_SYMBOL(kcs_bmc_handle_event); > >-struct kcs_bmc *kcs_bmc_ipmi_alloc(struct device *dev, int sizeof_priv, u32 channel); >-struct kcs_bmc *kcs_bmc_alloc(struct device *dev, int sizeof_priv, u32 channel) >+int kcs_bmc_ipmi_attach_cdev(struct kcs_bmc *kcs_bmc); Another declaration perhaps intended for kcs_bmc.h? >+int kcs_bmc_add_device(struct kcs_bmc *kcs_bmc) > { >- return kcs_bmc_ipmi_alloc(dev, sizeof_priv, channel); >+ return kcs_bmc_ipmi_attach_cdev(kcs_bmc); > } >-EXPORT_SYMBOL(kcs_bmc_alloc); >+EXPORT_SYMBOL(kcs_bmc_add_device); >+ >+int kcs_bmc_ipmi_detach_cdev(struct kcs_bmc *kcs_bmc); Here too. >+int kcs_bmc_remove_device(struct kcs_bmc *kcs_bmc) >+{ >+ return kcs_bmc_ipmi_detach_cdev(kcs_bmc); >+} >+EXPORT_SYMBOL(kcs_bmc_remove_device); > > MODULE_LICENSE("GPL v2"); > MODULE_AUTHOR("Haiyue Wang "); >diff --git a/drivers/char/ipmi/kcs_bmc.h b/drivers/char/ipmi/kcs_bmc.h >index febea0c8deb4..bf0ae327997f 100644 >--- a/drivers/char/ipmi/kcs_bmc.h >+++ b/drivers/char/ipmi/kcs_bmc.h >@@ -67,6 +67,8 @@ struct kcs_ioreg { > }; > > struct kcs_bmc { >+ struct device *dev; >+ > spinlock_t lock; > > u32 channel; >@@ -94,17 +96,11 @@ struct kcs_bmc { > u8 *kbuffer; > > struct miscdevice miscdev; >- >- unsigned long priv[]; > }; > >-static inline void *kcs_bmc_priv(struct kcs_bmc *kcs_bmc) >-{ >- return kcs_bmc->priv; >-} >- > int kcs_bmc_handle_event(struct kcs_bmc *kcs_bmc); >-struct kcs_bmc *kcs_bmc_alloc(struct device *dev, int sizeof_priv, u32 channel); >+int kcs_bmc_add_device(struct kcs_bmc *kcs_bmc); >+int kcs_bmc_remove_device(struct kcs_bmc *kcs_bmc); > > u8 kcs_bmc_read_data(struct kcs_bmc *kcs_bmc); > void kcs_bmc_write_data(struct kcs_bmc *kcs_bmc, u8 data); >diff --git a/drivers/char/ipmi/kcs_bmc_aspeed.c b/drivers/char/ipmi/kcs_bmc_aspeed.c >index 630cf095560e..0416ac78ce68 100644 >--- a/drivers/char/ipmi/kcs_bmc_aspeed.c >+++ b/drivers/char/ipmi/kcs_bmc_aspeed.c >@@ -61,6 +61,8 @@ > #define LPC_STR4 0x11C > > struct aspeed_kcs_bmc { >+ struct kcs_bmc kcs_bmc; >+ > struct regmap *map; > }; > >@@ -69,9 +71,14 @@ struct aspeed_kcs_of_ops { > int (*get_io_address)(struct platform_device *pdev); > }; > >+static inline struct aspeed_kcs_bmc *to_aspeed_kcs_bmc(struct kcs_bmc *kcs_bmc) >+{ >+ return container_of(kcs_bmc, struct aspeed_kcs_bmc, kcs_bmc); >+} >+ > static u8 aspeed_kcs_inb(struct kcs_bmc *kcs_bmc, u32 reg) > { >- struct aspeed_kcs_bmc *priv = kcs_bmc_priv(kcs_bmc); >+ struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc); > u32 val = 0; > int rc; > >@@ -83,7 +90,7 @@ static u8 aspeed_kcs_inb(struct kcs_bmc *kcs_bmc, u32 reg) > > static void aspeed_kcs_outb(struct kcs_bmc *kcs_bmc, u32 reg, u8 data) > { >- struct aspeed_kcs_bmc *priv = kcs_bmc_priv(kcs_bmc); >+ struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc); > int rc; > > rc = regmap_write(priv->map, reg, data); >@@ -92,7 +99,7 @@ static void aspeed_kcs_outb(struct kcs_bmc *kcs_bmc, u32 reg, u8 data) > > static void aspeed_kcs_updateb(struct kcs_bmc *kcs_bmc, u32 reg, u8 mask, u8 val) > { >- struct aspeed_kcs_bmc *priv = kcs_bmc_priv(kcs_bmc); >+ struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc); > int rc; > > rc = regmap_update_bits(priv->map, reg, mask, val); >@@ -114,7 +121,7 @@ static void aspeed_kcs_updateb(struct kcs_bmc *kcs_bmc, u32 reg, u8 mask, u8 val > */ > static void aspeed_kcs_set_address(struct kcs_bmc *kcs_bmc, u16 addr) > { >- struct aspeed_kcs_bmc *priv = kcs_bmc_priv(kcs_bmc); >+ struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc); > > switch (kcs_bmc->channel) { > case 1: >@@ -148,7 +155,7 @@ static void aspeed_kcs_set_address(struct kcs_bmc *kcs_bmc, u16 addr) > > static void aspeed_kcs_enable_channel(struct kcs_bmc *kcs_bmc, bool enable) > { >- struct aspeed_kcs_bmc *priv = kcs_bmc_priv(kcs_bmc); >+ struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc); > > switch (kcs_bmc->channel) { > case 1: >@@ -323,16 +330,16 @@ static int aspeed_kcs_of_v2_get_io_address(struct platform_device *pdev) > static int aspeed_kcs_probe(struct platform_device *pdev) > { > const struct aspeed_kcs_of_ops *ops; >- struct device *dev = &pdev->dev; >+ struct aspeed_kcs_bmc *priv; > struct kcs_bmc *kcs_bmc; > struct device_node *np; > int rc, channel, addr; > >- np = dev->of_node->parent; >+ np = pdev->dev.of_node->parent; > if (!of_device_is_compatible(np, "aspeed,ast2400-lpc-v2") && > !of_device_is_compatible(np, "aspeed,ast2500-lpc-v2") && > !of_device_is_compatible(np, "aspeed,ast2600-lpc-v2")) { >- dev_err(dev, "unsupported LPC device binding\n"); >+ dev_err(&pdev->dev, "unsupported LPC device binding\n"); > return -ENODEV; > } > ops = of_device_get_match_data(&pdev->dev); >@@ -343,18 +350,27 @@ static int aspeed_kcs_probe(struct platform_device *pdev) > if (channel < 0) > return channel; > >- kcs_bmc = kcs_bmc_alloc(&pdev->dev, sizeof(struct aspeed_kcs_bmc), channel); >- if (!kcs_bmc) >+ addr = ops->get_io_address(pdev); >+ if (addr < 0) >+ return addr; >+ >+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); >+ if (!priv) > return -ENOMEM; > >+ kcs_bmc = &priv->kcs_bmc; >+ kcs_bmc->dev = &pdev->dev; >+ kcs_bmc->channel = channel; > kcs_bmc->ioreg = ast_kcs_bmc_ioregs[channel - 1]; > kcs_bmc->io_inputb = aspeed_kcs_inb; > kcs_bmc->io_outputb = aspeed_kcs_outb; > kcs_bmc->io_updateb = aspeed_kcs_updateb; > >- addr = ops->get_io_address(pdev); >- if (addr < 0) >- return addr; >+ priv->map = syscon_node_to_regmap(pdev->dev.parent->of_node); >+ if (IS_ERR(priv->map)) { >+ dev_err(&pdev->dev, "Couldn't get regmap\n"); >+ return -ENODEV; >+ } The reanimated priv->map initialization I suspect wasn't meant to have been removed in the first place... > > aspeed_kcs_set_address(kcs_bmc, addr); > >@@ -362,29 +378,25 @@ static int aspeed_kcs_probe(struct platform_device *pdev) > if (rc) > return rc; > >- dev_set_drvdata(dev, kcs_bmc); >+ platform_set_drvdata(pdev, priv); > > aspeed_kcs_enable_channel(kcs_bmc, true); > >- rc = misc_register(&kcs_bmc->miscdev); >- if (rc) { >- dev_err(dev, "Unable to register device\n"); >+ rc = kcs_bmc_add_device(&priv->kcs_bmc); >+ if (rc < 0) > return rc; >- } > >- dev_dbg(&pdev->dev, >- "Probed KCS device %d (IDR=0x%x, ODR=0x%x, STR=0x%x)\n", >- kcs_bmc->channel, kcs_bmc->ioreg.idr, kcs_bmc->ioreg.odr, >- kcs_bmc->ioreg.str); >+ dev_info(&pdev->dev, "Initialised channel %d at 0x%x\n", kcs_bmc->channel, addr); Is the dbg->info change here intentional? (I have no particular objection if so, but it's often a change I make myself during testing/debugging and then forget to revert...) > > return 0; > } > > static int aspeed_kcs_remove(struct platform_device *pdev) > { >- struct kcs_bmc *kcs_bmc = dev_get_drvdata(&pdev->dev); >+ struct aspeed_kcs_bmc *priv = platform_get_drvdata(pdev); >+ struct kcs_bmc *kcs_bmc = &priv->kcs_bmc; > >- misc_deregister(&kcs_bmc->miscdev); >+ kcs_bmc_remove_device(kcs_bmc); Should we propagate the return value outward here? > > return 0; > } >diff --git a/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c b/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c >index 82c77994e481..0ca71c135a1a 100644 >--- a/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c >+++ b/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c >@@ -382,7 +382,7 @@ static int kcs_bmc_ipmi_release(struct inode *inode, struct file *filp) > return 0; > } > >-static const struct file_operations kcs_bmc_fops = { >+static const struct file_operations kcs_bmc_ipmi_fops = { > .owner = THIS_MODULE, > .open = kcs_bmc_ipmi_open, > .read = kcs_bmc_ipmi_read, >@@ -392,36 +392,58 @@ static const struct file_operations kcs_bmc_fops = { > .unlocked_ioctl = kcs_bmc_ipmi_ioctl, > }; > >-struct kcs_bmc *kcs_bmc_ipmi_alloc(struct device *dev, int sizeof_priv, u32 channel); >-struct kcs_bmc *kcs_bmc_ipmi_alloc(struct device *dev, int sizeof_priv, u32 channel) >+int kcs_bmc_ipmi_attach_cdev(struct kcs_bmc *kcs_bmc); Errant declaration again? >+int kcs_bmc_ipmi_attach_cdev(struct kcs_bmc *kcs_bmc) > { >- struct kcs_bmc *kcs_bmc; >- >- kcs_bmc = devm_kzalloc(dev, sizeof(*kcs_bmc) + sizeof_priv, GFP_KERNEL); >- if (!kcs_bmc) >- return NULL; >+ int rc; > > spin_lock_init(&kcs_bmc->lock); >- kcs_bmc->channel = channel; >- > mutex_init(&kcs_bmc->mutex); > init_waitqueue_head(&kcs_bmc->queue); > >- kcs_bmc->data_in = devm_kmalloc(dev, KCS_MSG_BUFSIZ, GFP_KERNEL); >- kcs_bmc->data_out = devm_kmalloc(dev, KCS_MSG_BUFSIZ, GFP_KERNEL); >- kcs_bmc->kbuffer = devm_kmalloc(dev, KCS_MSG_BUFSIZ, GFP_KERNEL); >+ kcs_bmc->data_in = devm_kmalloc(kcs_bmc->dev, KCS_MSG_BUFSIZ, GFP_KERNEL); >+ kcs_bmc->data_out = devm_kmalloc(kcs_bmc->dev, KCS_MSG_BUFSIZ, GFP_KERNEL); >+ kcs_bmc->kbuffer = devm_kmalloc(kcs_bmc->dev, KCS_MSG_BUFSIZ, GFP_KERNEL); > > kcs_bmc->miscdev.minor = MISC_DYNAMIC_MINOR; >- kcs_bmc->miscdev.name = devm_kasprintf(dev, GFP_KERNEL, "%s%u", >- DEVICE_NAME, channel); >+ kcs_bmc->miscdev.name = devm_kasprintf(kcs_bmc->dev, GFP_KERNEL, "%s%u", >+ DEVICE_NAME, kcs_bmc->channel); > if (!kcs_bmc->data_in || !kcs_bmc->data_out || !kcs_bmc->kbuffer || > !kcs_bmc->miscdev.name) >- return NULL; >- kcs_bmc->miscdev.fops = &kcs_bmc_fops; >+ return -ENOMEM; > >- return kcs_bmc; >+ kcs_bmc->miscdev.fops = &kcs_bmc_ipmi_fops; >+ >+ rc = misc_register(&kcs_bmc->miscdev); >+ if (rc) { >+ dev_err(kcs_bmc->dev, "Unable to register device: %d\n", rc); >+ return rc; >+ } >+ >+ dev_info(kcs_bmc->dev, "Initialised IPMI client for channel %d", kcs_bmc->channel); >+ >+ return 0; >+} >+EXPORT_SYMBOL(kcs_bmc_ipmi_attach_cdev); >+ >+int kcs_bmc_ipmi_detach_cdev(struct kcs_bmc *kcs_bmc); Same here. >+int kcs_bmc_ipmi_detach_cdev(struct kcs_bmc *kcs_bmc) >+{ >+ misc_deregister(&kcs_bmc->miscdev); >+ >+ spin_lock_irq(&kcs_bmc->lock); >+ kcs_bmc->running = 0; >+ kcs_bmc_ipmi_force_abort(kcs_bmc); >+ spin_unlock_irq(&kcs_bmc->lock); >+ >+ devm_kfree(kcs_bmc->dev, kcs_bmc->kbuffer); >+ devm_kfree(kcs_bmc->dev, kcs_bmc->data_out); >+ devm_kfree(kcs_bmc->dev, kcs_bmc->data_in); >+ devm_kfree(kcs_bmc->dev, kcs_bmc); >+ >+ return 0; > } >-EXPORT_SYMBOL(kcs_bmc_ipmi_alloc); >+EXPORT_SYMBOL(kcs_bmc_ipmi_detach_cdev); > > MODULE_LICENSE("GPL v2"); > MODULE_AUTHOR("Haiyue Wang "); >diff --git a/drivers/char/ipmi/kcs_bmc_npcm7xx.c b/drivers/char/ipmi/kcs_bmc_npcm7xx.c >index 1f44aadec9e8..5d017498dc69 100644 >--- a/drivers/char/ipmi/kcs_bmc_npcm7xx.c >+++ b/drivers/char/ipmi/kcs_bmc_npcm7xx.c >@@ -65,6 +65,8 @@ struct npcm7xx_kcs_reg { > }; > > struct npcm7xx_kcs_bmc { >+ struct kcs_bmc kcs_bmc; >+ > struct regmap *map; > > const struct npcm7xx_kcs_reg *reg; >@@ -76,9 +78,14 @@ static const struct npcm7xx_kcs_reg npcm7xx_kcs_reg_tbl[KCS_CHANNEL_MAX] = { > { .sts = KCS3ST, .dob = KCS3DO, .dib = KCS3DI, .ctl = KCS3CTL, .ie = KCS3IE }, > }; > >+static inline struct npcm7xx_kcs_bmc *to_npcm7xx_kcs_bmc(struct kcs_bmc *kcs_bmc) >+{ >+ return container_of(kcs_bmc, struct npcm7xx_kcs_bmc, kcs_bmc); >+} >+ > static u8 npcm7xx_kcs_inb(struct kcs_bmc *kcs_bmc, u32 reg) > { >- struct npcm7xx_kcs_bmc *priv = kcs_bmc_priv(kcs_bmc); >+ struct npcm7xx_kcs_bmc *priv = to_npcm7xx_kcs_bmc(kcs_bmc); > u32 val = 0; > int rc; > >@@ -90,7 +97,7 @@ static u8 npcm7xx_kcs_inb(struct kcs_bmc *kcs_bmc, u32 reg) > > static void npcm7xx_kcs_outb(struct kcs_bmc *kcs_bmc, u32 reg, u8 data) > { >- struct npcm7xx_kcs_bmc *priv = kcs_bmc_priv(kcs_bmc); >+ struct npcm7xx_kcs_bmc *priv = to_npcm7xx_kcs_bmc(kcs_bmc); > int rc; > > rc = regmap_write(priv->map, reg, data); >@@ -99,7 +106,7 @@ static void npcm7xx_kcs_outb(struct kcs_bmc *kcs_bmc, u32 reg, u8 data) > > static void npcm7xx_kcs_updateb(struct kcs_bmc *kcs_bmc, u32 reg, u8 mask, u8 data) > { >- struct npcm7xx_kcs_bmc *priv = kcs_bmc_priv(kcs_bmc); >+ struct npcm7xx_kcs_bmc *priv = to_npcm7xx_kcs_bmc(kcs_bmc); > int rc; > > rc = regmap_update_bits(priv->map, reg, mask, data); >@@ -108,7 +115,7 @@ static void npcm7xx_kcs_updateb(struct kcs_bmc *kcs_bmc, u32 reg, u8 mask, u8 da > > static void npcm7xx_kcs_enable_channel(struct kcs_bmc *kcs_bmc, bool enable) > { >- struct npcm7xx_kcs_bmc *priv = kcs_bmc_priv(kcs_bmc); >+ struct npcm7xx_kcs_bmc *priv = to_npcm7xx_kcs_bmc(kcs_bmc); > > regmap_update_bits(priv->map, priv->reg->ctl, KCS_CTL_IBFIE, > enable ? KCS_CTL_IBFIE : 0); >@@ -155,11 +162,10 @@ static int npcm7xx_kcs_probe(struct platform_device *pdev) > return -ENODEV; > } > >- kcs_bmc = kcs_bmc_alloc(dev, sizeof(*priv), chan); >- if (!kcs_bmc) >+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); >+ if (!priv) > return -ENOMEM; > >- priv = kcs_bmc_priv(kcs_bmc); > priv->map = syscon_node_to_regmap(dev->parent->of_node); > if (IS_ERR(priv->map)) { > dev_err(dev, "Couldn't get regmap\n"); >@@ -167,6 +173,9 @@ static int npcm7xx_kcs_probe(struct platform_device *pdev) > } > priv->reg = &npcm7xx_kcs_reg_tbl[chan - 1]; > >+ kcs_bmc = &priv->kcs_bmc; >+ kcs_bmc->dev = &pdev->dev; >+ kcs_bmc->channel = chan; > kcs_bmc->ioreg.idr = priv->reg->dib; > kcs_bmc->ioreg.odr = priv->reg->dob; > kcs_bmc->ioreg.str = priv->reg->sts; >@@ -174,31 +183,27 @@ static int npcm7xx_kcs_probe(struct platform_device *pdev) > kcs_bmc->io_outputb = npcm7xx_kcs_outb; > kcs_bmc->io_updateb = npcm7xx_kcs_updateb; > >- dev_set_drvdata(dev, kcs_bmc); >+ platform_set_drvdata(pdev, priv); > > npcm7xx_kcs_enable_channel(kcs_bmc, true); > rc = npcm7xx_kcs_config_irq(kcs_bmc, pdev); > if (rc) > return rc; > >- rc = misc_register(&kcs_bmc->miscdev); >- if (rc) { >- dev_err(dev, "Unable to register device\n"); >- return rc; >- } > > pr_info("channel=%u idr=0x%x odr=0x%x str=0x%x\n", > chan, > kcs_bmc->ioreg.idr, kcs_bmc->ioreg.odr, kcs_bmc->ioreg.str); > >- return 0; >+ return kcs_bmc_add_device(kcs_bmc); > } > > static int npcm7xx_kcs_remove(struct platform_device *pdev) > { >- struct kcs_bmc *kcs_bmc = dev_get_drvdata(&pdev->dev); >+ struct npcm7xx_kcs_bmc *priv = platform_get_drvdata(pdev); >+ struct kcs_bmc *kcs_bmc = &priv->kcs_bmc; > >- misc_deregister(&kcs_bmc->miscdev); >+ kcs_bmc_remove_device(kcs_bmc); As with the corresponding aspeed code, should we propagate the return value here? > > return 0; > } >-- >2.27.0 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel