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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 32C9EC433F5 for ; Thu, 28 Apr 2022 12:41:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346476AbiD1Mop (ORCPT ); Thu, 28 Apr 2022 08:44:45 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47558 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1346408AbiD1Moa (ORCPT ); Thu, 28 Apr 2022 08:44:30 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9DD4122508; Thu, 28 Apr 2022 05:41:11 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 07C7262021; Thu, 28 Apr 2022 12:41:11 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id D9F75C385A0; Thu, 28 Apr 2022 12:41:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1651149670; bh=H/3u/CcX1d8+DE8rBf/zqVY14MTJASBdkzizEGaA2gY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=FOs9EWCWVLCEqaXtG5zXHSqfUuRcmK39Gie4feWGqoBCMY4fVcqunoM19Ioq6fRIc byprs6MT8/Jv0cCj152QLuSmmdW2NKovaPcLqTSz0eljrwzmXO/Tu1YLeOZZcM+JLD fYMjWMp6IbARFh8usL2icsBGfCEQOLuqwjToGTbg= Date: Thu, 28 Apr 2022 14:41:07 +0200 From: Greg Kroah-Hartman To: Muhammad Usama Anjum Cc: "Rafael J. Wysocki" , Len Brown , Hans de Goede , Mark Gross , Collabora Kernel ML , groeck@chromium.org, bleung@chromium.org, dtor@chromium.org, gwendal@chromium.org, vbendeb@chromium.org, andy@infradead.org, Ayman Bagabas , Benjamin Tissoires , =?utf-8?B?Qmxhxb4=?= Hrastnik , Darren Hart , Dmitry Torokhov , Jeremy Soller , Mattias Jacobsson <2pi@mok.nu>, Mauro Carvalho Chehab , Rajat Jain , Srinivas Pandruvada , platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org, "Rafael J . Wysocki" , Enric Balletbo i Serra Subject: Re: [PATCH v8] platform: x86: Add ChromeOS ACPI device driver Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org On Thu, Apr 28, 2022 at 02:40:08PM +0200, Greg Kroah-Hartman wrote: > On Thu, Apr 28, 2022 at 05:24:04PM +0500, Muhammad Usama Anjum wrote: > > On 4/24/22 1:43 PM, Greg Kroah-Hartman wrote: > > > On Fri, Apr 15, 2022 at 10:08:15PM +0500, Muhammad Usama Anjum wrote: > > >> + i = 0; > > >> + list_for_each_entry(aag, &chromeos_acpi.groups, list) { > > >> + chromeos_acpi.dev_groups[i] = &aag->group; > > >> + i++; > > >> + } > > >> + > > >> + ret = sysfs_create_groups(&dev->kobj, chromeos_acpi.dev_groups); > > > > > > You have raced with userspace and lost here :( > > > > > Sorry, What does it mean exactly? > > Long old post that describes the issue in detail is here: > http://www.kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly/ > > > > Use the default groups pointer in the platform driver for this, and use > > > the is_visible() callback to know to show, or not show, the attribute > > > instead of building up dynamic lists of attributes at runtime. That > > > will save you lots of crazy logic and housekeeping _AND_ userspace tools > > > will work properly as well. > > > > > > > Driver has the 2 kinds of attributes: > > > > A) Attributes which are always there. For example, CHSW and HWIDs etc. > > They can be easily shows via dev_groups pointer in platform driver. > > Great. > > > B) Attribute groups which vary between 0 to N. N is platform dependent > > and can be determined at runtime. For example, GPIO attribute group > > which have 4 sub attributes in it: > > > > Group GPIO.0 --> attributes GPIO.0, GPIO.1, GPIO.2 and GPIO.3 > > Group GPIO.1 --> attributes GPIO.0, GPIO.1, GPIO.2 and GPIO.3 > > ... > > Group GPIO.N --> attributes GPIO.0, GPIO.1, GPIO.2 and GPIO.3 > > > > My Chromebook has 2 GPIO attribute groups while I've found logs of a > > Chromebook which has 7 GPIO groups. > > > > Why these groups cannot be defined at compile time (Shortcomings): > > > > 1) We don't know the total GPIO groups. > > Possible solution: Determine GPIO groups' number at run time and define > > attributes at run time. > > What is the max number of groups you can ever have? 10? 100? 1000? > Pick a high number, define them all (macros make this easy), and then > only enable the ones that you need at runtime. > > > 2) We cannot determine from attribute name that this group will be > > visible or not as is_visible doesn't provide information about its group > > name. > > umode_t (*is_visible)(struct kobject *, struct attribute *, int); > > Look at the attribute pointer. That's all you care about. Compare it > to a real pointer and away you go! Also remember, each group has a is_visible function, so you know what group this is implicitly. thanks, greg k-h