From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: MIME-Version: 1.0 In-Reply-To: <1500555312.2354.75.camel@pengutronix.de> References: <20170719152646.25903-1-p.zabel@pengutronix.de> <20170719211515.46a1196c@windsurf> <1500543415.2354.37.camel@pengutronix.de> <20170720123640.43c2ce01@windsurf> <1500555312.2354.75.camel@pengutronix.de> From: Dmitry Torokhov Date: Thu, 20 Jul 2017 13:46:17 -0700 Message-ID: Subject: Re: [PATCH 000/102] Convert drivers to explicit reset API Content-Type: text/plain; charset="UTF-8" To: Philipp Zabel Cc: Thomas Petazzoni , lkml , Andrew Lunn , Prashant Gaikwad , Heiko Stuebner , Peter Chen , Linus Walleij , DRI , Marc Dietrich , Rakesh Iyer , Peter Meerwald-Stadler , linux-clk@vger.kernel.org, Wim Van Sebroeck , Wolfram Sang , Xinliang Liu , Chanwoo Choi , Alan Stern , Jiri Slaby , Michael Turquette , Guenter Roeck , Ohad Ben-Cohen , "linux-pm@vger.kernel.org" , Thomas Gleixner , Vincent Abriou , Bin Liu , Greg Kroah-Hartman , USB list , linux-wireless , Ralf Baechle , linux-spi , linux-crypto@vger.kernel.org, Tejun Heo , "alsa-devel@alsa-project.org" , "linux-doc@vger.kernel.org" , David Airlie , nouveau@lists.freedesktop.org, Philippe Cornu , Kalle Valo , Laxman Dewangan , Corentin Labbe , Linux I2C , LINUXWATCHDOG , Boris Brezillon , Lars-Peter Clausen , =?UTF-8?Q?Emilio_L=C3=B3pez?= , Daniel Lezcano , Jon Hunter , "open list:ARM/Rockchip SoC..." , MyungJoo Ham , Ben Skeggs , Yisen Zhuang , "linux-media@vger.kernel.org" , Richard Zhu , Alexandre Torgue , Mathias Nyman , "linux-arm-msm@vger.kernel.org" , Joachim Eastwood , "linux-gpio@vger.kernel.org" , "linux-mips@linux-mips.org" , Bjorn Helgaas , Giuseppe Cavallaro , "linux-arm-kernel@lists.infradead.org" , Patrice Chotard , Stanimir Varbanov , Kyungmin Park , Maxime Coquelin , Hartmut Knaack , Jonathan Cameron , Ulf Hansson , linux-iio@vger.kernel.org, Linux PCI , Shawn Lin , linux-tegra@vger.kernel.org, linux-mtd@lists.infradead.org, Benjamin Gaignard , Florian Fainelli , Jonathan Corbet , Xinwei Kong , ath10k@lists.infradead.org, Kishon Vijay Abraham I , Chen-Yu Tsai , "linux-input@vger.kernel.org" , "linux-pwm@vger.kernel.org" , Chen Feng , Mark Brown , Dan Williams , Felipe Balbi , Salil Mehta , linux-mmc , Liam Girdwood , Thierry Reding , Cyrille Pitchen , Srinivas Kandagatla , Maxime Ripard , Brian Norris , "David S. Miller" , linux-remoteproc@vger.kernel.org, Bjorn Andersson , "linux-ide@vger.kernel.org" , Lee Jones , devel@driverdev.osuosl.org, Yannick Fertre , Ryder Lee , Herbert Xu , Richard Weinberger , Jaehoon Chung , Marek Vasut , linux-serial , Zhang Rui , Alan Tull , John Youn , Eduardo Valentin , dmaengine@vger.kernel.org, linux-mediatek@lists.infradead.org, Matthias Brugger , Mark Yao , Moritz Fischer , Vivien Didelot , netdev , Peter De Schrijver , Stephen Boyd , Adrian Hunter , Vinod Koul , Rongrong Zou , linux-fpga@vger.kernel.org, David Woodhouse , Lucas Stach List-ID: On Thu, Jul 20, 2017 at 5:55 AM, Philipp Zabel wrote: > Hi Thomas, > > On Thu, 2017-07-20 at 12:36 +0200, Thomas Petazzoni wrote: >> Hello, >> >> On Thu, 20 Jul 2017 11:36:55 +0200, Philipp Zabel wrote: >> >> > > I don't know if it has been discussed in the past, so forgive me if it >> > > has been. Have you considered adding a "int flags" argument to the >> > > existing reset_control_get_*() functions, rather than introducing >> > > separate exclusive variants ? >> > > >> > > Indeed, with a "int flags" argument you could in the future add more >> > > variants/behaviors without actually multiplying the number of >> > > functions. Something like the "flags" argument for request_irq() for >> > > example. >> > >> > I can't find the discussion right now, but I remember we had talked >> > about this in the past. >> > Behind the scenes, all the inline API functions already call common >> > entry points with flags (well, currently separate bool parameters for >> > shared and optional). >> > One reason against exposing those as an int flags in the user facing API >> > is the possibility to accidentally provide a wrong value. >> >> This is a quite strange argument. You could also accidentally use the >> wrong variant of the function, just like you could use the wrong flag. > > You can't accidentally use no flag at all or a completely bogus value > with the "plethora of inline functions" variant. > >> Once again, the next time you have another parameter for those reset >> functions, beyond the exclusive/shared variant, you will multiply again >> by two the number of functions ? You already have the exclusive/shared >> and optional/mandatory variants, so 4 variants. When you'll add a new >> parameter, you'll have 8 variants. Doesn't seem really good. > > I'd rather avoid adding more variants, if possible. The complexity > increases regardless of whether the API is expressed as a bunch of > functions or as a single function with a bunch of flags. > >> What about reset_control_get(struct device *, const char *, int flags) >> to replace all those variants ? > > While I like how this looks, unfortunately (devm_)reset_control_get > already exists without the flags, so we can't change to that with a > gentle transition. This was done for gpiod_get() and its flags argument with horrifying #define-ry, which thankfully was completely hidden from users. -- Dmitry From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dmitry Torokhov Subject: Re: [PATCH 000/102] Convert drivers to explicit reset API Date: Thu, 20 Jul 2017 13:46:17 -0700 Message-ID: References: <20170719152646.25903-1-p.zabel@pengutronix.de> <20170719211515.46a1196c@windsurf> <1500543415.2354.37.camel@pengutronix.de> <20170720123640.43c2ce01@windsurf> <1500555312.2354.75.camel@pengutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Cc: Thomas Petazzoni , lkml , Andrew Lunn , Prashant Gaikwad , Heiko Stuebner , Peter Chen , Linus Walleij , DRI , Marc Dietrich , Rakesh Iyer , Peter Meerwald-Stadler , linux-clk@vger.kernel.org, Wim Van Sebroeck , Wolfram Sang , Xinliang Liu , Chanwoo Choi , Alan Stern , Jiri Slaby , Michael Turquette , Guenter Roeck , To: Philipp Zabel Return-path: In-Reply-To: <1500555312.2354.75.camel@pengutronix.de> Sender: linux-mips-bounce@linux-mips.org Errors-to: linux-mips-bounce@linux-mips.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-subscribe: List-owner: List-post: List-archive: List-Id: linux-crypto.vger.kernel.org On Thu, Jul 20, 2017 at 5:55 AM, Philipp Zabel wrote: > Hi Thomas, > > On Thu, 2017-07-20 at 12:36 +0200, Thomas Petazzoni wrote: >> Hello, >> >> On Thu, 20 Jul 2017 11:36:55 +0200, Philipp Zabel wrote: >> >> > > I don't know if it has been discussed in the past, so forgive me if it >> > > has been. Have you considered adding a "int flags" argument to the >> > > existing reset_control_get_*() functions, rather than introducing >> > > separate exclusive variants ? >> > > >> > > Indeed, with a "int flags" argument you could in the future add more >> > > variants/behaviors without actually multiplying the number of >> > > functions. Something like the "flags" argument for request_irq() for >> > > example. >> > >> > I can't find the discussion right now, but I remember we had talked >> > about this in the past. >> > Behind the scenes, all the inline API functions already call common >> > entry points with flags (well, currently separate bool parameters for >> > shared and optional). >> > One reason against exposing those as an int flags in the user facing API >> > is the possibility to accidentally provide a wrong value. >> >> This is a quite strange argument. You could also accidentally use the >> wrong variant of the function, just like you could use the wrong flag. > > You can't accidentally use no flag at all or a completely bogus value > with the "plethora of inline functions" variant. > >> Once again, the next time you have another parameter for those reset >> functions, beyond the exclusive/shared variant, you will multiply again >> by two the number of functions ? You already have the exclusive/shared >> and optional/mandatory variants, so 4 variants. When you'll add a new >> parameter, you'll have 8 variants. Doesn't seem really good. > > I'd rather avoid adding more variants, if possible. The complexity > increases regardless of whether the API is expressed as a bunch of > functions or as a single function with a bunch of flags. > >> What about reset_control_get(struct device *, const char *, int flags) >> to replace all those variants ? > > While I like how this looks, unfortunately (devm_)reset_control_get > already exists without the flags, so we can't change to that with a > gentle transition. This was done for gpiod_get() and its flags argument with horrifying #define-ry, which thankfully was completely hidden from users. -- Dmitry From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dmitry Torokhov Subject: Re: [PATCH 000/102] Convert drivers to explicit reset API Date: Thu, 20 Jul 2017 13:46:17 -0700 Message-ID: References: <20170719152646.25903-1-p.zabel@pengutronix.de> <20170719211515.46a1196c@windsurf> <1500543415.2354.37.camel@pengutronix.de> <20170720123640.43c2ce01@windsurf> <1500555312.2354.75.camel@pengutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Return-path: In-Reply-To: <1500555312.2354.75.camel@pengutronix.de> Sender: linux-mips-bounce@linux-mips.org Errors-to: linux-mips-bounce@linux-mips.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-subscribe: List-owner: List-post: List-archive: To: Philipp Zabel Cc: Thomas Petazzoni , lkml , Andrew Lunn , Prashant Gaikwad , Heiko Stuebner , Peter Chen , Linus Walleij , DRI , Marc Dietrich , Rakesh Iyer , Peter Meerwald-Stadler , linux-clk@vger.kernel.org, Wim Van Sebroeck , Wolfram Sang , Xinliang Liu , Chanwoo Choi , Alan Stern , Jiri Slaby , Michael Turquette , Guenter Roeck List-Id: linux-tegra@vger.kernel.org On Thu, Jul 20, 2017 at 5:55 AM, Philipp Zabel wrote: > Hi Thomas, > > On Thu, 2017-07-20 at 12:36 +0200, Thomas Petazzoni wrote: >> Hello, >> >> On Thu, 20 Jul 2017 11:36:55 +0200, Philipp Zabel wrote: >> >> > > I don't know if it has been discussed in the past, so forgive me if it >> > > has been. Have you considered adding a "int flags" argument to the >> > > existing reset_control_get_*() functions, rather than introducing >> > > separate exclusive variants ? >> > > >> > > Indeed, with a "int flags" argument you could in the future add more >> > > variants/behaviors without actually multiplying the number of >> > > functions. Something like the "flags" argument for request_irq() for >> > > example. >> > >> > I can't find the discussion right now, but I remember we had talked >> > about this in the past. >> > Behind the scenes, all the inline API functions already call common >> > entry points with flags (well, currently separate bool parameters for >> > shared and optional). >> > One reason against exposing those as an int flags in the user facing API >> > is the possibility to accidentally provide a wrong value. >> >> This is a quite strange argument. You could also accidentally use the >> wrong variant of the function, just like you could use the wrong flag. > > You can't accidentally use no flag at all or a completely bogus value > with the "plethora of inline functions" variant. > >> Once again, the next time you have another parameter for those reset >> functions, beyond the exclusive/shared variant, you will multiply again >> by two the number of functions ? You already have the exclusive/shared >> and optional/mandatory variants, so 4 variants. When you'll add a new >> parameter, you'll have 8 variants. Doesn't seem really good. > > I'd rather avoid adding more variants, if possible. The complexity > increases regardless of whether the API is expressed as a bunch of > functions or as a single function with a bunch of flags. > >> What about reset_control_get(struct device *, const char *, int flags) >> to replace all those variants ? > > While I like how this looks, unfortunately (devm_)reset_control_get > already exists without the flags, so we can't change to that with a > gentle transition. This was done for gpiod_get() and its flags argument with horrifying #define-ry, which thankfully was completely hidden from users. -- Dmitry From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: MIME-Version: 1.0 In-Reply-To: <1500555312.2354.75.camel@pengutronix.de> References: <20170719152646.25903-1-p.zabel@pengutronix.de> <20170719211515.46a1196c@windsurf> <1500543415.2354.37.camel@pengutronix.de> <20170720123640.43c2ce01@windsurf> <1500555312.2354.75.camel@pengutronix.de> From: Dmitry Torokhov Date: Thu, 20 Jul 2017 13:46:17 -0700 Message-ID: Subject: Re: [PATCH 000/102] Convert drivers to explicit reset API List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "ath10k" Errors-To: ath10k-bounces+kvalo=adurom.com@lists.infradead.org To: Philipp Zabel Cc: Andrew Lunn , Prashant Gaikwad , Heiko Stuebner , Peter Chen , Linus Walleij , DRI , Marc Dietrich , Rakesh Iyer , Peter Meerwald-Stadler , linux-clk@vger.kernel.org, Wolfram Sang , Xinliang Liu , Chanwoo Choi , Alan Stern , Jiri Slaby , Michael Turquette , Guenter Roeck , Ohad Ben-Cohen , "linux-pm@vger.kernel.org" , Thomas Gleixner , Vincent Abriou , Bin Liu , Greg Kroah-Hartman , USB list , linux-wireless , lkml , Ralf Baechle , linux-spi , linux-crypto@vger.kernel.org, Tejun Heo , "alsa-devel@alsa-project.org" , "linux-doc@vger.kernel.org" , David Airlie , nouveau@lists.freedesktop.org, Philippe Cornu , Wim Van Sebroeck , Laxman Dewangan , Corentin Labbe , Linux I2C , Alexandre Torgue , Boris Brezillon , Lars-Peter Clausen , =?UTF-8?Q?Emilio_L=C3=B3pez?= , Daniel Lezcano , Jon Hunter , "open list:ARM/Rockchip SoC..." , MyungJoo Ham , Ben Skeggs , Yisen Zhuang , "linux-media@vger.kernel.org" , Richard Zhu , LINUXWATCHDOG , Mathias Nyman , "linux-arm-msm@vger.kernel.org" , Joachim Eastwood , "linux-gpio@vger.kernel.org" , "linux-mips@linux-mips.org" , Bjorn Helgaas , Giuseppe Cavallaro , "linux-arm-kernel@lists.infradead.org" , Thomas Petazzoni , Patrice Chotard , Stanimir Varbanov , Kyungmin Park , Maxime Coquelin , Hartmut Knaack , Jonathan Cameron , Ulf Hansson , linux-iio@vger.kernel.org, Linux PCI , Shawn Lin , Matthias Brugger , linux-mtd@lists.infradead.org, Benjamin Gaignard , Florian Fainelli , Jonathan Corbet , Chen-Yu Tsai , ath10k@lists.infradead.org, Kishon Vijay Abraham I , Xinwei Kong , "linux-input@vger.kernel.org" , "linux-pwm@vger.kernel.org" , Chen Feng , Mark Brown , Dan Williams , Felipe Balbi , Salil Mehta , linux-mmc , Kalle Valo , Thierry Reding , Cyrille Pitchen , Srinivas Kandagatla , Maxime Ripard , Brian Norris , "David S. Miller" , Liam Girdwood , linux-remoteproc@vger.kernel.org, Bjorn Andersson , "linux-ide@vger.kernel.org" , Lee Jones , devel@driverdev.osuosl.org, Yannick Fertre , Ryder Lee , Herbert Xu , Richard Weinberger , Jaehoon Chung , Marek Vasut , linux-serial , Zhang Rui , Alan Tull , John Youn , Eduardo Valentin , Rongrong Zou , linux-mediatek@lists.infradead.org, linux-tegra@vger.kernel.org, Mark Yao , Moritz Fischer , Vivien Didelot , netdev , Peter De Schrijver , Stephen Boyd , Adrian Hunter , Vinod Koul , dmaengine@vger.kernel.org, linux-fpga@vger.kernel.org, David Woodhouse , Lucas Stach On Thu, Jul 20, 2017 at 5:55 AM, Philipp Zabel wrote: > Hi Thomas, > > On Thu, 2017-07-20 at 12:36 +0200, Thomas Petazzoni wrote: >> Hello, >> >> On Thu, 20 Jul 2017 11:36:55 +0200, Philipp Zabel wrote: >> >> > > I don't know if it has been discussed in the past, so forgive me if it >> > > has been. Have you considered adding a "int flags" argument to the >> > > existing reset_control_get_*() functions, rather than introducing >> > > separate exclusive variants ? >> > > >> > > Indeed, with a "int flags" argument you could in the future add more >> > > variants/behaviors without actually multiplying the number of >> > > functions. Something like the "flags" argument for request_irq() for >> > > example. >> > >> > I can't find the discussion right now, but I remember we had talked >> > about this in the past. >> > Behind the scenes, all the inline API functions already call common >> > entry points with flags (well, currently separate bool parameters for >> > shared and optional). >> > One reason against exposing those as an int flags in the user facing API >> > is the possibility to accidentally provide a wrong value. >> >> This is a quite strange argument. You could also accidentally use the >> wrong variant of the function, just like you could use the wrong flag. > > You can't accidentally use no flag at all or a completely bogus value > with the "plethora of inline functions" variant. > >> Once again, the next time you have another parameter for those reset >> functions, beyond the exclusive/shared variant, you will multiply again >> by two the number of functions ? You already have the exclusive/shared >> and optional/mandatory variants, so 4 variants. When you'll add a new >> parameter, you'll have 8 variants. Doesn't seem really good. > > I'd rather avoid adding more variants, if possible. The complexity > increases regardless of whether the API is expressed as a bunch of > functions or as a single function with a bunch of flags. > >> What about reset_control_get(struct device *, const char *, int flags) >> to replace all those variants ? > > While I like how this looks, unfortunately (devm_)reset_control_get > already exists without the flags, so we can't change to that with a > gentle transition. This was done for gpiod_get() and its flags argument with horrifying #define-ry, which thankfully was completely hidden from users. -- Dmitry _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k