Hi Jakub On Fri, 6 Oct 2023 15:08:10 -0700 Jakub Kicinski <kuba@kernel.org> wrote:
On Thu, 28 Sep 2023 09:06:42 +0200 Herve Codina wrote:
+menu "Framer Subsystem" + +config GENERIC_FRAMER + bool "Framer Core" + help + Generic Framer support. + A framer is a component in charge of an E1/T1 line interface. + Connected usually to a TDM bus, it converts TDM frames to/from E1/T1 + frames. It also provides information related to the E1/T1 line. + Used with HDLC, the network can be reached through the E1/T1 line. + + This framework is designed to provide a generic interface for framer + devices present in the kernel. This layer will have the generic + API by which framer drivers can create framer using the framer + framework and framer users can obtain reference to the framer. + All the users of this framework should select this config.
maybe make the menu a menuconfig with info about framers but hide the GENERIC_FRAMER symbol? The driver 'select' it anyway, what's the point of prompting the user..
Yes, I will change in the next iteration.
+ if (WARN_ON(!dev)) + return ERR_PTR(-EINVAL);
no defensive programming, let the kernel crash
Will be changed.
+ ret = framer_pm_runtime_get_sync(framer); + if (ret < 0 && ret != -EOPNOTSUPP) + goto err_pm_sync; + + ret = 0; /* Override possible ret == -EOPNOTSUPP */
This looks pointless given that ret is either overwritten or not used later on
Indeed. Will be removed in the next iteration.
+ mutex_lock(&framer->mutex); + if (framer->power_count == 0 && framer->ops->power_on) { + ret = framer->ops->power_on(framer); + if (ret < 0) { + dev_err(&framer->dev, "framer poweron failed --> %d\n", ret); + goto err_pwr_on; + } + } + ++framer->power_count; + mutex_unlock(&framer->mutex); + return 0;
Best regards, Hervé