Date:   Mon, 13 Mar 2023 17:16:59 +0100
From:   Greg KH <gregkh@linuxfoundation.org>
To:     VaibhaavRam.TL@microchip.com
Cc:     arnd@arndb.de, Kumaravel.Thiagarajan@microchip.com,
        linux-kernel@vger.kernel.org, UNGLinuxDriver@microchip.com,
        Tharunkumar.Pasumarthi@microchip.com
Subject: Re: [PATCH v7 char-misc-next] misc: microchip: pci1xxxx: Add
 OTP/EEPROM driver for the pci1xxxx switch
Message-ID: <ZA9Me2d9IsoYGifp@kroah.com>
References: <20230303170426.353-1-vaibhaavram.tl@microchip.com>
 <ZAm7PDSIUN5HMxUu@kroah.com>
 <DM4PR11MB62385D0541F7AF8EC7E9F6CE97B99@DM4PR11MB6238.namprd11.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <DM4PR11MB62385D0541F7AF8EC7E9F6CE97B99@DM4PR11MB6238.namprd11.prod.outlook.com>
Precedence: bulk

On Mon, Mar 13, 2023 at 04:01:15PM +0000, VaibhaavRam.TL@microchip.com wrote:
> > Again, default group will handle this automatically, you should never 
> > need to call a sysfs_*() call from a driver.  Otherwise something is usually very wrong.
> 
> Are you recommending similar to this snippet?
> 
> static struct bin_attribute *pci1xxxx_bin_attributes[] = {
> 	&pci1xxxx_otp_attr,
> 	&pci1xxxx_eeprom_attr
> 	NULL,
> };
> 
> static const struct attribute_group pci1xxxx_bin_attributes_group = {
> 	.bin_attrs = pci1xxxx_bin_attributes,
> };
> ..
> ..
> auxiliary_device.device.attribute_group = pci1xxxx_bin_attributes_group

Yes.

> This creates sysfs for both EEPROM and OTP at once and handles for its removal, right?
> But, In this case, I have to check whether EEPROM is responsive and only then create sysfs for it.
> 
> Can you please provide some guidance, on how to handle this situation without using sysfs_*().

Use the "is_visible" callback in your group to tell the driver core if
the specific attribute needs to be created or not.

thanks,

greg k-h