Re: [PATCH net-next 06/11] dpll: netlink: Add DPLL framework base functions

From: Jiri Pirko <jiri@resnulli.us>
To: Vadim Fedorenko <vadim.fedorenko@linux.dev>
Cc: Jakub Kicinski <kuba@kernel.org>,
	Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>,
	Jonathan Lemon <jonathan.lemon@gmail.com>,
	Paolo Abeni <pabeni@redhat.com>,
	Milena Olech <milena.olech@intel.com>,
	Michal Michalik <michal.michalik@intel.com>,
	linux-arm-kernel@lists.infradead.org, poros@redhat.com,
	mschmidt@redhat.com, netdev@vger.kernel.org,
	linux-clk@vger.kernel.org, Bart Van Assche <bvanassche@acm.org>
Subject: Re: [PATCH net-next 06/11] dpll: netlink: Add DPLL framework base functions
Date: Tue, 1 Aug 2023 14:23:26 +0200	[thread overview]
Message-ID: <ZMj5PrwZK3Fat7fT@nanopsycho> (raw)
In-Reply-To: <20230720091903.297066-7-vadim.fedorenko@linux.dev>

Thu, Jul 20, 2023 at 11:18:58AM CEST, vadim.fedorenko@linux.dev wrote:

[...]

>+
>+static int
>+dpll_pin_parent_device_set(struct dpll_pin *pin, struct nlattr *parent_nest,
>+			   struct netlink_ext_ack *extack)
>+{
>+	struct nlattr *tb[DPLL_A_MAX + 1];
>+	enum dpll_pin_direction direction;
>+	enum dpll_pin_state state;
>+	struct dpll_pin_ref *ref;
>+	struct dpll_device *dpll;
>+	u32 pdpll_idx, prio;
>+	int ret;
>+
>+	nla_parse_nested(tb, DPLL_A_MAX, parent_nest,
>+			 NULL, extack);

Please pass proper policy instead of NULL here.
dpll_pin_parent_device_nl_policy

>+	if (!tb[DPLL_A_ID]) {
>+		NL_SET_ERR_MSG(extack, "device parent id expected");
>+		return -EINVAL;
>+	}
>+	pdpll_idx = nla_get_u32(tb[DPLL_A_ID]);
>+	dpll = xa_load(&dpll_device_xa, pdpll_idx);
>+	if (!dpll)
>+		return -EINVAL;
>+	ref = xa_load(&pin->dpll_refs, dpll->device_idx);
>+	ASSERT_NOT_NULL(ref);
>+	if (tb[DPLL_A_PIN_STATE]) {
>+		state = nla_get_u8(tb[DPLL_A_PIN_STATE]);
>+		ret = dpll_pin_state_set(dpll, pin, state, extack);
>+		if (ret)
>+			return ret;
>+	}
>+	if (tb[DPLL_A_PIN_PRIO]) {
>+		prio = nla_get_u8(tb[DPLL_A_PIN_PRIO]);
>+		ret = dpll_pin_prio_set(dpll, pin, prio, extack);
>+		if (ret)
>+			return ret;
>+	}
>+	if (tb[DPLL_A_PIN_DIRECTION]) {
>+		direction = nla_get_u8(tb[DPLL_A_PIN_DIRECTION]);
>+		ret = dpll_pin_direction_set(pin, dpll, direction, extack);
>+		if (ret)
>+			return ret;
>+	}
>+	return 0;
>+}
>+
>+static int
>+dpll_pin_parent_pin_set(struct dpll_pin *pin, struct nlattr *parent_nest,
>+			struct netlink_ext_ack *extack)
>+{
>+	struct nlattr *tb[DPLL_A_MAX + 1];
>+	enum dpll_pin_state state;
>+	u32 ppin_idx;
>+	int ret;
>+
>+	nla_parse_nested(tb, DPLL_A_MAX, parent_nest,
>+			 NULL, extack);

Please pass proper policy instead of NULL here.
dpll_pin_parent_pin_nl_policy

>+	if (!tb[DPLL_A_PIN_ID]) {
>+		NL_SET_ERR_MSG(extack, "parent pin id expected");
>+		return -EINVAL;
>+	}
>+	ppin_idx = nla_get_u32(tb[DPLL_A_PIN_ID]);
>+	state = nla_get_u8(tb[DPLL_A_PIN_STATE]);
>+	ret = dpll_pin_on_pin_state_set(pin, ppin_idx, state, extack);
>+	if (ret)
>+		return ret;
>+
>+	return 0;
>+}

[...]