From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Rosin Subject: Re: [PATCH v1 1/2] eeprom: at24: convert magic numbers to structs. Date: Fri, 8 Dec 2017 16:39:09 +0100 Message-ID: References: <1512576272-25563-1-git-send-email-svendev@arcx.com> <1512576272-25563-2-git-send-email-svendev@arcx.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org To: Sven Van Asbroeck , Bartosz Golaszewski Cc: Sven Van Asbroeck , Wolfram Sang , nsekhar@ti.com, Sakari Ailus , Javier Martinez Canillas , Divagar Mohandass , devicetree@vger.kernel.org, Linux Kernel Mailing List , linux-i2c List-Id: devicetree@vger.kernel.org On 2017-12-08 16:28, Sven Van Asbroeck wrote: > Bartosz wrote: >> Just make it accept two parameters: the string and the data object >> name. For example: >> >> AT24_OF_DEVICE_ID("atmel,24c01", 24c01); >> > > I don't want to do too much bikeshedding, > but one last comment. > > Is it a good idea to have duplicated information? > Do you think the risk of a typo mismatch is acceptable? E.g. > AT24_OF_DEVICE_ID("atmel,24c01", 24c02); > > At least the original syntax is not susceptible to this problem, > but of course as you say, it hides the of_ compatible string: > AT24_OF_DEVICE_ID(atmel, 24c01); > /* hides 'atmel,24c01' */ It's nice to be able to grep for compatible strings. $.02 /peda