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..

+	if (WARN_ON(!dev))
+		return ERR_PTR(-EINVAL);

no defensive programming, let the kernel crash

+	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

+	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;