From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 09AD3372ED0; Fri, 13 Mar 2026 21:01:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773435662; cv=none; b=VK0gY1I+pu6WoD8bIuG/LLTPKopv5cGEzw2wzXMC4AynNK8CgJwwOEqigkigDlLcpi/fKtrWZXm2AVKDjIvpTqpx1GVKgfRiGc7l0rH6CZGTv/Lbfx1/8udYZqRdX7aZWABGZQxtrHo8hp478cN0Tq4l37TR+5tk/dqJnifpOWo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773435662; c=relaxed/simple; bh=O1nNclFDbyjxv8qi+UO7kEi9GWMfQEet1G/iJQKNSuc=; h=Mime-Version:Content-Type:Date:Message-Id:To:From:Subject:Cc: References:In-Reply-To; b=ZLcBVZjDCqqi6o1pE6N419bc32P7pvvn9rO7VwutpAIVp91iMYfCWeL5x8DZE37AplGu8J5fqNmY8Nn6q0jc6JlBcMWDOwwz1p2ezBa07q4q9ukZz8PTjxrsMH1+pvt5qNWT6Kp0dlxXLK9IwtdzYQ0ljpNgdaBRVMo9wf4V8GA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=D60pN7n6; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="D60pN7n6" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0E7B6C19421; Fri, 13 Mar 2026 21:00:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1773435661; bh=O1nNclFDbyjxv8qi+UO7kEi9GWMfQEet1G/iJQKNSuc=; h=Date:To:From:Subject:Cc:References:In-Reply-To:From; b=D60pN7n6xCiMmcp3DkouG5BJQPAqjLqpjWsWKm+eEugVKQ5TZldFQIJXq1h3YMsmi tV83t/LH1xOh+B9DjVLiLJz8WxXP5EAk/RJVUk5gsVWhGIuL0mKQ6ZtcObZEmsfW8S VHV2QBVE+7aYD277iItBjI/hI7Vvq4wr1ngLHgGp1vHD6oDjlT+hPFTw7SICS8Cbfr pTAMPFLTsGCmuWuW2Xiqls+SP/2p1uX8tQgdRU1WOGvfcvWPoRMCdgvNyVvLfgcaUA VJxGtsGL+nlgZDLchLAdeQKG2jmZ50CX8BXimDtnaClH0fuxKSOU4TOJWSkggs2VY8 2TiPAYz4wZ/EA== Precedence: bulk X-Mailing-List: rust-for-linux@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Fri, 13 Mar 2026 22:00:54 +0100 Message-Id: To: "Markus Probst via B4 Relay" From: "Danilo Krummrich" Subject: Re: [PATCH v3 7/7] leds: add synology microp led driver Cc: , "Lee Jones" , "Rob Herring" , "Krzysztof Kozlowski" , "Conor Dooley" , "Greg Kroah-Hartman" , "Miguel Ojeda" , "Boqun Feng" , "Gary Guo" , =?utf-8?q?Bj=C3=B6rn_Roy_Baron?= , "Benno Lossin" , "Andreas Hindborg" , "Alice Ryhl" , "Trevor Gross" , "Rafael J. Wysocki" , "Igor Korotin" , "Daniel Almeida" , "Bjorn Helgaas" , =?utf-8?q?Krzysztof_Wilczy=C5=84ski?= , "Pavel Machek" , "Len Brown" , "Robert Moore" , , , , , , , , References: <20260313-synology_microp_initial-v3-0-ad6ac463a201@posteo.de> <20260313-synology_microp_initial-v3-7-ad6ac463a201@posteo.de> In-Reply-To: <20260313-synology_microp_initial-v3-7-ad6ac463a201@posteo.de> On Fri Mar 13, 2026 at 8:03 PM CET, Markus Probst via B4 Relay wrote: > +impl Command { > + fn write(self, dev: &platform::Device) -> Result { > + // SAFETY: Since we have no of and no acpi match table, we assum= e this is a mfd sub-device > + // and our parent is a serial device bus device, bound to the sy= nology microp core driver. > + let parent =3D unsafe { dev.as_ref().parent_unchecked::>() }; Despite being accurate description, "assume" is not what you want to read f= or a safety justification. :) We don't want to directly access the serial device from this driver. Instea= d, there should be an abstraction layer of the resource you are accessing. If this would be I2C or SPI you would request the regmap of the parent at t= his point, e.g. dev.parent().regmap("led_registers") Now, this is a serial device, but regmap still works perfectly fine for thi= s case. It even allows you to ensure from the MFD driver to restrict the LED driver of sending commands that are not LED specific by exposing a LED spec= ific regmap. Additionally, if you need additional locking etc. it can all be don= e within the regmap implementation, so you entirely avoid custom APIs. I'm not sure how common regmap is for serial devices to be honest, but apparently there are drivers doing this and I don't really see a reason aga= inst it. For instance, there is drivers/iio/imu/bno055/, which is a chip that works = on both serial and I2C busses and fully abstracts this fact with regmap. In Rust a regmap will probably become a backend of the generic I/O infrastructure we are working on, which will also allow you to use the register!() infrastructure, etc. register!() and some other generic I/O improvements will land this cycle, I= /O projections are more likely to land next cycle. > + parent.write_all( > + match self { > + Self::Power(State::On) =3D> &[0x34], > + Self::Power(State::Blink) =3D> &[0x35], > + Self::Power(State::Off) =3D> &[0x36], > + > + Self::Status(_, State::Off) =3D> &[0x37], > + Self::Status(StatusLedColor::Green, State::On) =3D> &[0x= 38], > + Self::Status(StatusLedColor::Green, State::Blink) =3D> &= [0x39], > + Self::Status(StatusLedColor::Orange, State::On) =3D> &[0= x3A], > + Self::Status(StatusLedColor::Orange, State::Blink) =3D> = &[0x3B], > + > + Self::Alert(State::On) =3D> &[0x4C, 0x41, 0x31], > + Self::Alert(State::Blink) =3D> &[0x4C, 0x41, 0x32], > + Self::Alert(State::Off) =3D> &[0x4C, 0x41, 0x33], > + > + Self::Usb(State::On) =3D> &[0x40], > + Self::Usb(State::Blink) =3D> &[0x41], > + Self::Usb(State::Off) =3D> &[0x42], > + }, > + serdev::Timeout::Max, > + )?; > + Ok(()) > + } > +}