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=-10.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 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 6E2BCC433E0 for ; Thu, 7 Jan 2021 14:29:40 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 2972922EBF for ; Thu, 7 Jan 2021 14:29:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2972922EBF Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org 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=merlin.20170209; h=Sender:Content-Type: Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:References: To:Subject:From:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=bXYoeFBK6GcI+Q3ckRzWb2k7xIzJKQ+vmF+zHAdE/Sw=; b=AnKp5MonnhjqaaYFRN0PRs5gW 60859PZkYtr+N4vYqBRthCQ/3gFeIU4Qj0vFjTC+nDNgcmFSsmzs+dKDTFONtkJ6xEaNzCDj6Hczf cGbhyeyO5ffo8cCPj8kZolsSJmUVPR13z2JU5MACdR/tfxe8Eh4+R4cwrnnU6zTyAxU1NAqMSgWtz JKVTGAUjxudo3S/kHkc783yRYx8ZcL0wetFUFSYIOAyMd6tDK6ooAXJxgpRTkKvgOvteciVjGKG9G z5X7uh5uNFR3cuGXRcGH0OYVj/7gOXrkDTrifTrdps6yjWzEG/uA5XxTzMH+4SEcAatZ/+7giSK5G /y3sNlFkg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kxWGt-0001lp-B6; Thu, 07 Jan 2021 14:28:15 +0000 Received: from mail-il1-x136.google.com ([2607:f8b0:4864:20::136]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kxWGr-0001kt-6j for linux-arm-kernel@lists.infradead.org; Thu, 07 Jan 2021 14:28:14 +0000 Received: by mail-il1-x136.google.com with SMTP id x15so6917414ilq.1 for ; Thu, 07 Jan 2021 06:28:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:subject:to:cc:references:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=EE6w3AuO9Q5DdDEsKJsefaa+Y3hE+KAyib/x3tsZfSY=; b=ugAb8czjOTWziVNg3ceJ79t+TpgzIYQ7zZMsaf9uXqr6ZEKBM8ZJiyKdz4NRIz1jyo kndR7TqgII+oHjkFyh75odOqNtpFLyJ8TQ5S/mKyZ0vvz3GYR4ve+v1L45Ey/ukCztGX nczGIL5kdwwDzHLSpEaQdqEq7J8Mx2SCyYzoJLsgUgaH86c+VMQxFWzUW4r2u803w1YA V9wrW7+QuUiDwG4VtOpvECWVtKuymRJ40Ak4b4PToQxV9kLEOAsmA7ccDEyFxaualMld dG+XeUnq7YBk+3sap0pIs6OJsYsvxK1h6yXQNcYh4W6itycbaCE6BB2Iluw+c5Lk+i/a E7gw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:subject:to:cc:references:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=EE6w3AuO9Q5DdDEsKJsefaa+Y3hE+KAyib/x3tsZfSY=; b=aIhbH8Xw+2RrjEs8uUbMiiT3c8X6ZCUijZLDb8upwxMsyd4BeIG/8b1iWGTKAtCqsb 7iis/XNDL/zwmdoe/1QgbTz8js2Y4IJXWcIk48zUbuhGqUyzARcsy1J0JIzyCfGLEaRq kSE3AGkIDBafPhp7a2Sk4ElGhRGKXJX0btvbSFmAOjmDv+dfvMWjCQfejU5eDHyATOL5 3Z8p8xwTxgNvZ5I47HaLqziJkyTO29dn3gm+BNQ7oBywdeoL+tkaCVngjR5A3EeJRr6D PKjJaLjA095sd0KmukY+kRRVWN5RDUN2lnDKoCbWE0ybcPfgAqg04Jg78s3u8nvERoTK POMA== X-Gm-Message-State: AOAM531A6tTLRGGKQ3Foj5JUNYtHv8cgtJzRMyrKTvZlYRKx9vv3g7IX ZmlEvEse2kJIzgIuoJ3mjOkWSQ== X-Google-Smtp-Source: ABdhPJxxIZR97qv2b/mWYtH/zkIITpXZ3J8s9lmvfMKkW3Y5G7uADanyCbzJjs1TvI62wpfvDXfQTw== X-Received: by 2002:a92:ba55:: with SMTP id o82mr2372256ili.202.1610029689186; Thu, 07 Jan 2021 06:28:09 -0800 (PST) Received: from [192.168.1.93] (pool-71-163-245-5.washdc.fios.verizon.net. [71.163.245.5]) by smtp.gmail.com with ESMTPSA id o18sm3505496ioa.39.2021.01.07.06.28.07 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 07 Jan 2021 06:28:08 -0800 (PST) From: Thara Gopinath Subject: Re: [PATCH v4 37/37] firmware: arm_scmi: add dynamic scmi devices creation To: Cristian Marussi , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org References: <20210106201610.26538-1-cristian.marussi@arm.com> <20210106201610.26538-38-cristian.marussi@arm.com> Message-ID: <50434a02-0fe0-50f3-1529-51ab8a0cc1f3@linaro.org> Date: Thu, 7 Jan 2021 09:28:07 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <20210106201610.26538-38-cristian.marussi@arm.com> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210107_092813_374159_61B921CA X-CRM114-Status: GOOD ( 34.70 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: f.fainelli@gmail.com, vincent.guittot@linaro.org, sudeep.holla@arm.com, james.quinlan@broadcom.com, Jonathan.Cameron@Huawei.com, souvik.chakravarty@arm.com, etienne.carriere@linaro.org, lukasz.luba@arm.com Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Christian, On 1/6/21 3:16 PM, Cristian Marussi wrote: > Having added the support for SCMI protocols as modules in order to let > vendors extend the SCMI core with their own additions it seems odd to > then force SCMI drivers built on top to use a static device table to > declare their devices since this way any new SCMI drivers addition > would need the core SCMI device table to be updated too. > > Remove the static core device table and let SCMI drivers to simply declare > which device/protocol pair they need at initialization time: the core will > then take care to generate such devices dynamically during platform > initialization or at module loading time, as long as the requested > underlying protocol is defined in the DT. > > Signed-off-by: Cristian Marussi > --- [snip] > > -static inline void > -scmi_create_protocol_devices(struct device_node *np, struct scmi_info *info, > - int prot_id) > + for (; rdev; rdev = rdev->next) > + scmi_create_protocol_device(np, info, prot_id, > + rdev->id_table->name); > +} > + > +/** > + * scmi_request_protocol_device - Helper to request a device > + * > + * @id_table: A protocol/name pair descriptor for the device to be created. > + * > + * This helper let an SCMI driver request specific devices identified by the > + * @id_table to be created for each active SCMI instance. > + * > + * The requested device name MUST NOT be already existent for any protocol; > + * at first the freshly requested @id_table is annotated in the IDR table > + * @scmi_requested_devices, then a matching device is created for each already > + * active SCMI instance. (if any) > + * > + * This way the requested device is created straight-away for all the already > + * initialized(probed) SCMI instances (handles) but it remains instead pending > + * for creation if the requesting SCMI driver is loaded before some instance > + * and related transports was available: when such late SCMI instance is probed > + * it will take care to scan the list of pending requested devices and create > + * those on its own (see @scmi_create_protocol_devices and its enclosing loop) > + * > + * Return: 0 on Success > + */ > +int scmi_request_protocol_device(const struct scmi_device_id *id_table) > { > - int loop, cnt; > + int ret = 0; > + unsigned int id = 0; > + struct scmi_requested_dev *rdev, *proto_rdev = NULL; > + struct scmi_info *info; > > - for (loop = 0; loop < ARRAY_SIZE(devnames); loop++) { > - if (devnames[loop].protocol_id != prot_id) > - continue; > + pr_debug("Requesting SCMI device (%s) for protocol %x\n", > + id_table->name, id_table->protocol_id); > > - for (cnt = 0; cnt < ARRAY_SIZE(devnames[loop].names); cnt++) { > - const char *name = devnames[loop].names[cnt]; > + /* > + * Search for the matching protocol rdev list and then search > + * of any existent equally named device...fails if any duplicate found. > + */ > + mutex_lock(&scmi_requested_devices_mutex); > + idr_for_each_entry(&scmi_requested_devices, rdev, id) { > + if (rdev->id_table->protocol_id == id_table->protocol_id) > + proto_rdev = rdev; > + for (; rdev; rdev = rdev->next) { > + if (!strcmp(rdev->id_table->name, id_table->name)) { > + pr_err("Ignoring duplicate request [%d] %s\n", > + rdev->id_table->protocol_id, > + rdev->id_table->name); > + ret = -EINVAL; > + goto out; > + } Shouldn't there be proto_rdev = rdev here as well ? > + } > + } > + > + /* > + * No duplicate found for requested id_table, so let's create a new > + * requested device entry for this new valid request. > + */ > + rdev = kzalloc(sizeof(*rdev), GFP_KERNEL); > + if (!rdev) { > + ret = -ENOMEM; > + goto out; > + } > + rdev->id_table = id_table; > + > + /* > + * Append the new requested device table descriptor to the head of the > + * related protocol chain, eventually creating such chain if not already > + * there. > + */ > + if (!proto_rdev) { > + ret = idr_alloc(&scmi_requested_devices, (void *)rdev, > + rdev->id_table->protocol_id, > + rdev->id_table->protocol_id + 1, GFP_KERNEL); > + if (ret != rdev->id_table->protocol_id) { > + pr_err("Failed to save SCMI device - ret:%d\n", ret); > + kfree(rdev); > + ret = -EINVAL; > + goto out; > + } > + ret = 0; > + } else { > + proto_rdev->next = rdev; > + } > + > + /* > + * Now effectively create and initialize the requested device for every > + * already initialized SCMI instance which has registered the requested > + * protocol as a valid active one: i.e. defined in DT and supported by > + * current platform FW. > + */ > + mutex_lock(&scmi_list_mutex); > + list_for_each_entry(info, &scmi_list, node) { > + struct device_node *child; > + > + child = idr_find(&info->active_protocols, > + id_table->protocol_id); > + if (child) { > + struct scmi_device *sdev; > + > + sdev = scmi_get_protocol_device(child, info, > + id_table->protocol_id, > + id_table->name); > + /* Set handle if not already set (device existed) */ > + if (sdev && !sdev->handle) > + sdev->handle = scmi_handle_get_from_info(info); > + } else { > + dev_err(info->dev, > + "Failed. SCMI protocol %d not active.\n", > + id_table->protocol_id); > + } > + } > + mutex_unlock(&scmi_list_mutex); > + > +out: > + mutex_unlock(&scmi_requested_devices_mutex); > + > + return ret; > +} > + > +/** > + * scmi_unrequest_protocol_device - Helper to unrequest a device > + * > + * @id_table: A protocol/name pair descriptor for the device to be unrequested. > + * > + * An helper to let an SCMI driver release its request about devices; note that > + * devices are created and initialized once the first SCMI driver request them > + * but they destroyed only on SCMI core unloading/unbinding. > + * > + * The current SCMI transport layer uses such devices as internal references and > + * as such they could be shared as same transport between multiple drivers so > + * that cannot be safely destroyed till the whole SCMI stack is removed. > + * (unless adding further burden of refcounting.) > + */ > +void scmi_unrequest_protocol_device(const struct scmi_device_id *id_table) > +{ > + struct scmi_requested_dev *victim, *prev, *head; > + > + pr_debug("Unrequesting SCMI device (%s) for protocol %x\n", > + id_table->name, id_table->protocol_id); > > - if (name) > - scmi_create_protocol_device(np, info, prot_id, > - name); > + head = idr_find(&scmi_requested_devices, id_table->protocol_id); > + if (!head) > + return; > + > + /* > + * Scan the protocol list of requested device name searching > + * for the victim. > + */ > + victim = head; > + for (prev = victim; victim; prev = victim, victim = victim->next) The initial assignment for the for loop is wrong. With this when you break prev will be equal to victim. You want prev to be the one pointing to the victim. Or am I missing something? -- Warm Regards Thara _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel