From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3863CC25B08 for ; Mon, 8 Aug 2022 19:09:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244253AbiHHTJT (ORCPT ); Mon, 8 Aug 2022 15:09:19 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59750 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S244214AbiHHTJS (ORCPT ); Mon, 8 Aug 2022 15:09:18 -0400 Received: from mail-oa1-x30.google.com (mail-oa1-x30.google.com [IPv6:2001:4860:4864:20::30]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B485E19C27 for ; Mon, 8 Aug 2022 12:09:17 -0700 (PDT) Received: by mail-oa1-x30.google.com with SMTP id 586e51a60fabf-10ea30a098bso11574413fac.8 for ; Mon, 08 Aug 2022 12:09:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=content-transfer-encoding:cc:to:subject:message-id:date:user-agent :from:references:in-reply-to:mime-version:from:to:cc; bh=4l9i4XlbJ+5mmKTKFgWECtlXoLq4GTjM56DrApQ3zMc=; b=WXnldZ6iOBHOrddzUwoQRxgam/JBzwyx71/OjpN+S0nPQiCYjfVHsJak5hpqrKugdI sz6jv8KOXTrlq54eF2VH5UdV7Ril0q3866QyHAGP7YLmCKcZCn4etUrQKuvkYj3xEFbR IZrXnD+WpPDpLEeukyzKmLWA37sTIZb1PT660= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:user-agent :from:references:in-reply-to:mime-version:x-gm-message-state:from:to :cc; bh=4l9i4XlbJ+5mmKTKFgWECtlXoLq4GTjM56DrApQ3zMc=; b=oF7neBWX0eAJaPjeAkBgd0kP5Rvd8DOus2zzFVmQX+VazgEKbIi1usPBLK9qH+vdSJ 9BDsBuvB9XiFDTk/3I+BGDWTmt8jzu4AD/g5PEQSVOCwiOtqLGAXNW4t0uPvUkIqnWaO UaCrLliBfnxjZ5VNe9sA+f4/RFFH6gOO9obA/gwVatmp5nDQid0yjCgUdEzxHVSM6G6H APTuGc2M0FojBZTwZ6xkUgindmBD/cTVv9x+H/9p7FvNzOBYiTb8Nn9DqcWeAhTnBDCW KolzIfkf2UrDE1J7NWX/DTusPZn7jV9hv01PK1redYih79dg2he3oHV28DIPlLUnQ5dl 3dOw== X-Gm-Message-State: ACgBeo36WUVaRpgx98EiE8B0OJ9zjJPMAm7VKEC2JQMtrm/vBE0V/htw z042hqcWh5grUp2B2H9l6hBrE0+1kBWcLbd1znh6UQ== X-Google-Smtp-Source: AA6agR5mBX/UZhs7o/pIA55lLsdbPNZPdEc7fmUEWwPyqYNUtxSkx623SBa9QmGpQH54M7wu3v9gLmG6aUnL+1cjbiU= X-Received: by 2002:a05:6870:9a17:b0:e9:3d1:f91a with SMTP id fo23-20020a0568709a1700b000e903d1f91amr9076366oab.44.1659985756290; Mon, 08 Aug 2022 12:09:16 -0700 (PDT) Received: from 753933720722 named unknown by gmailapi.google.com with HTTPREST; Mon, 8 Aug 2022 14:09:15 -0500 MIME-Version: 1.0 In-Reply-To: References: <0481d3cc-4bb9-4969-0232-76ba57ff260d@quicinc.com> <52039cd1-4390-7abb-d296-0eb7ac0c3b15@quicinc.com> From: Stephen Boyd User-Agent: alot/0.10 Date: Mon, 8 Aug 2022 14:09:15 -0500 Message-ID: Subject: Re: [PATCH V15 6/9] mfd: pm8008: Use i2c_new_dummy_device() API To: Lee Jones , Satya Priya Kakitapalli Cc: Mark Brown , Bjorn Andersson , Rob Herring , Liam Girdwood , linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, quic_collinsd@quicinc.com, quic_subbaram@quicinc.com, quic_jprakash@quicinc.com, Lee Jones Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org Quoting Lee Jones (2022-08-05 03:51:39) > On Tue, 02 Aug 2022, Satya Priya Kakitapalli (Temp) wrote: > > > > > On 7/27/2022 6:49 AM, Stephen Boyd wrote: > > > Quoting Satya Priya Kakitapalli (Temp) (2022-07-21 23:31:16) > > > > =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 regulato= r-name =3D "pm8008_l6"; > > > > =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 }; > > > > > > > > =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 pm8008_l7: ldo7@4600 { > > > > =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 reg =3D = <0x4600>; > > > > =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 regulato= r-name =3D "pm8008_l7"; > > > > =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 }; > > > > =C2=A0=C2=A0 =C2=A0}; > > > > }; > > > > > > > > > > > > Stephen/Mark, Please do let me know if you are OK with this design. > > > > > > > I was happy with the previous version of the DT node. That had one no= de > > > for the "pm8008 chip", which is important because it really is one > > > package. Why isn't that possible to implement and also register i2c > > > devices on the i2c bus for the second address? > > If devices have different addresses, they should have their own nodes, no= ? There are nodes for the devices at different addresses in the design we had settled on earlier. pm8008: pmic@8 { compatible =3D "qcom,pm8008"; reg =3D <0x8>; #address-cells =3D <2>; #size-cells =3D <0>; #interrupt-cells =3D <2>; pm8008_l1: ldo1@1,4000 { compatible =3D "qcom,pm8008-regulator"; reg =3D <0x1 0x4000>; regulator-name =3D "pm8008_ldo1"; }; ... }; pmic@8 is the i2c device at i2c address 8. ldo1@1,4000 is the i2c device at address 9 (8 + 1) with control for ldo1 at register offset 0x4000. I think your concern is more about the fact that the regulator sub-nodes are platform device drivers instead of i2c device drivers. I'm not an i2c expert but from what I can tell we only describe one i2c address in DT and then do something like this to describe the other i2c addresses when one physical chip responds to multiple addresses on the i2c bus. See i2c_new_dummy_device() and i2c_new_ancillary_device() kerneldoc for slightly more background. It may need some modifications to the i2c core to make the regulator nodes into i2c devices. I suspect the qcom,pm8008 i2c driver needs to look at the 'reg' property and translate that to the parent node's reg property (8) plus the first cell (1) to determine the i2c device's final i2c address. Then the i2c core needs to register i2c devices that are bound to the lifetime of the "primary" i2c device (pmic@8). The driver for the regulator can parse the second cell of the reg property to determine the register offset within that i2c address to use to control the regulator. That would make it possible to create an i2c device for each regulator node, but I don't think that is correct because the second reg property isn't an i2c address, it's a register offset inside the i2c address. It sort of looks like we need to use i2c_new_ancillary_device() here. IF we did that the DT would look like this: pm8008: pmic@8 { compatible =3D "qcom,pm8008"; reg =3D <0x8>, <0x9>; reg-names =3D "core", "regulators"; #address-cells =3D <2>; #size-cells =3D <0>; #interrupt-cells =3D <2>; pm8008_l1: ldo1@1,4000 { compatible =3D "qcom,pm8008-regulator"; reg =3D <0x1 0x4000>; regulator-name =3D "pm8008_ldo1"; }; ... }; And a dummy i2c device would be created for i2c address 0x9 bound to the dummy i2c driver when we called i2c_new_ancillary_device() with "regulators" for the name. The binding of the dummy driver is preventing us from binding another i2c driver to the i2c address. Why can't we call i2c_new_client_device() but avoid binding a dummy driver to it like i2c_new_ancillary_device() does? If that can be done, then the single i2c device at 0x9 can be a pm8008-regulators (plural) device that probes a single i2c driver that parses the subnodes looking for regulator nodes. Note: There is really one i2c device at address 0x9, that corresponds to the regulators, but in DT we need to have one node per regulator so we can configure constraints.