Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) client-ip=2620:137:e000::3:8;
Message-ID: <0cb645c7-f3c5-e4bb-7686-2a83d32274bb@axentia.se>
Date:   Fri, 20 Oct 2023 16:28:12 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.0
Subject: Re: [PATCH v3] mux: mmio: use reg property when parent device is not
 a syscon
Content-Language: sv-SE, en-US
To:     Andrew Davis <afd@ti.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Nishanth Menon <nm@ti.com>,
        Vignesh Raghavendra <vigneshr@ti.com>
Cc:     devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20230911151030.71100-1-afd@ti.com>
From:   Peter Rosin <peda@axentia.se>
In-Reply-To: <20230911151030.71100-1-afd@ti.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dVZNR3BtU0RCV0Q1UksyM1NaNGJHSGY4SG40QVExVmMvTnNHVjV6b3c1T2ph?=
 =?utf-8?B?YlhLUnJzNWF3ZTk1REtVaU9IU1YyNks2U0lkT21xbGR2QlRyTDZTOXZLWmVz?=
 =?utf-8?B?V2s0TmE5ellHL0tVR3cwMmFOTk1UVlc5N2t3Tzdvd1FvbG1jQVMvYlFlWTdF?=
 =?utf-8?B?bGtYTWRpZ0NJbER2Um1WYTlRQiszL0xFRi80MlRlVGhjZUZESFdEanBwdTJQ?=
 =?utf-8?B?VFZHZktPMHVnN2RLVHNQbThYWFE4dTkyU0taZ0FjU3pocWVuTTZLQ2xjV0VG?=
 =?utf-8?B?T0luMVNyK2k5SVhUd0x5TFBPOWRzZ0YzT0g1am13WXFHTDJxcFdKUmNqYlJS?=
 =?utf-8?B?WHd4b1llRnIrOG9TaTBVVmZWS3Fva2ZlYWU5d0tUUmJ1aTdnb1BDdVI2ZTlX?=
 =?utf-8?B?eitabEZWRkxnOGdvem04U0pybmFLSi9rNUtGTGRpd1hmSGRhM3FPQ1pQYU5Z?=
 =?utf-8?B?bU5ISklyT0VXU2ltUkJ3bDhXUkxnU3lyWWNHNm1lbDFpV21wTmxmckh3dlYv?=
 =?utf-8?B?d2dvOFhUR0thV2RKQzYvM3hFNXJuOXVlL0FzakYvTnRXRC9FZ25nbkdNT090?=
 =?utf-8?B?dDRCUzhBZDJVZ0xXRG9lSjgyWVpWR1U2dlQ0aWlaN0ZwenVwSG5vY0JmUk9P?=
 =?utf-8?B?Qmh1OVVhMTZ3NXFCbUpMWHUxMzcwaWk5NUtwdHRYWDdGQk81TXBNSk5MT3NR?=
 =?utf-8?B?T2JybzZVc2FYRHhyMnA4L1VHVlNFWTZSRGRHeDlsc1k3cU54eWtXbzZhM1Nn?=
 =?utf-8?B?SzFFb2pwODQrWDI1eC9MWldOdkRyWmJjTXprb3Y2WmlBdVlMOWE0REc1elN1?=
 =?utf-8?B?RXdQdUoxb201clZOSEhMZUplM1BZVG5DdzAxL2lnazRIMnNBQmhaVndvR3pG?=
 =?utf-8?B?eWdWVE1WeENSQ0oxUmhROWNsR2JKdFcyMUZDRVpMY0FVMEp2bkxVQjJmUWh6?=
 =?utf-8?B?aDFXOGVOWG9EQlVjdVdkTkZUMXJjY05idW1xcWpEeHpwWGV3eEh1RnozWGRh?=
 =?utf-8?B?TWY2U3JnZWZCYlg0VnFLV3JLMXRsaGVVYjdOaGRwZkQ1Um1FQXV4WmRvbkcy?=
 =?utf-8?B?bEJMNDc5SnZEdmRSS0VnTGVkd2gxMWRUYlBWM2tQZjh0WTZkZ2tYVmtJTXUw?=
 =?utf-8?B?a0RzV2tXSis2WldPaHlFZ1FyRTlrclRFUGFxQ2hEc1JRR3JGUUp0UDEyNFU3?=
 =?utf-8?B?SWpQWlp6eDRmc2grUjFjV1JrYU5uODdzdGtWRWd3bG1xaWFhNFV6bEV6SFYy?=
 =?utf-8?B?SlhERFQwRVl1NTZ0Y2JuTmk1OGpQdkdmMVJQck8vdlpJU1ZKQ0VNemZEazcw?=
 =?utf-8?B?UHFHbnRwdnVDVXdNY3BWeDVORDRkZ2dsUHk5TkFKdUhLdkFFL2luNENjMkhT?=
 =?utf-8?B?N01waVJtR29pcGZlK3QvZFJrMVhSRDUxMlZRc3N6bVV4Ym5mSDZRczVPUEpI?=
 =?utf-8?B?cHdlNEg2cUp2dExOQXF3QVBqb2VnOUlWQ1k4TnI2dmhMVmY5bGdrWTBmUUc0?=
 =?utf-8?B?cGdXTnRIek1YM0VYcHFMa3JQUTlXUUhLY1JEK1RrbVZlTGp6dXZRZHhJbXBT?=
 =?utf-8?B?azUrR0pLL0hacDR1LzJ4c1pSWlYvbHNYTFNuaThlWHp3eno3L3hiSFdBY051?=
 =?utf-8?B?U1hlZURXVUNNZ1RraFpUNGpOT2d4UXZ6czVVOTRsamFZcjNleEdTS0hEcmsr?=
 =?utf-8?B?YVk0NXBmbnZWelV4UzZYVjd0ZXF6dExRMEc3NlFwbVp3L3owU3JzYUVpdkds?=
 =?utf-8?B?OHVhelQwMnF3Q083VjgrWlE3a2hWQUVHSGVsSVh6T1g5bnljVkNTd0VseEU0?=
 =?utf-8?B?SzltNGFTVXlVZThKRHB5OU9jMDdEL3Q1d0lzejIrUURUV0RzWk1mMys0V2tD?=
 =?utf-8?B?VGVDV2RKS3ZIYnp4SEpuYVdHUnpXT1NGZ2hHT2sxY1VsaWNvZTlTUU5ZcDVx?=
 =?utf-8?B?TVliVGp2VjIzRU8zWFRVR0dScmhWUEdHcVVlOGJocXZ3VEt0dXVaMnRWdG5P?=
 =?utf-8?B?Vmh4ekRBUUp5dWNWMmRWcnBqeE9laUtUd1d1RDNwenZYTFZGUzNqVFhUTXRv?=
 =?utf-8?B?ZkRac0syMW9ZZDBZWmVQbUkxUFNIOFNZdGt6VjNEblNnZEo1eFc1N2RTYkFw?=
 =?utf-8?Q?qEC/BU+ruBRjSufcTa0Z6VWbA?=
X-OriginatorOrg: axentia.se
X-MS-Exchange-CrossTenant-Network-Message-Id: f73c996d-c4a5-4814-5388-08dbd178cb7b
X-MS-Exchange-CrossTenant-AuthSource: AM0PR02MB4436.eurprd02.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Oct 2023 14:28:14.4921
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4ee68585-03e1-4785-942a-df9c1871a234
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: XPAJQBc/VP3SdFrct/ruM++8nI75mhGZDjf9fuQTzoUqFIrpTLPC2ltWv28PibDs
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DU2PR02MB10332
X-Spam-Status: No, score=-4.2 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (fry.vger.email [0.0.0.0]); Fri, 20 Oct 2023 07:28:26 -0700 (PDT)

Hi!

2023-09-11 at 17:10, Andrew Davis wrote:
> The DT binding for the reg-mux compatible states it can be used when the
> "parent device of mux controller is not syscon device". It also allows
> for a reg property. When the reg property is provided, use that to
> identify the address space for this mux. If not provided fallback to
> using the parent device as a regmap provider.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>
> Reviewed-by: Nishanth Menon <nm@ti.com>
> ---
> 
> Changes from v2:
>  - Rebased on v6.6-rc1
> 
> Changes from v1:
>  - Flip logic as suggested in v1[0]
> 
> [0] https://lore.kernel.org/lkml/1c27d9d4-b1cc-c158-90f7-f7e47e02c424@ti.com/T/
> 
>  drivers/mux/mmio.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mux/mmio.c b/drivers/mux/mmio.c
> index fd1d121a584ba..b6095b7853ed2 100644
> --- a/drivers/mux/mmio.c
> +++ b/drivers/mux/mmio.c
> @@ -44,10 +44,13 @@ static int mux_mmio_probe(struct platform_device *pdev)
>  	int ret;
>  	int i;
>  
> -	if (of_device_is_compatible(np, "mmio-mux"))
> +	if (of_device_is_compatible(np, "mmio-mux")) {
>  		regmap = syscon_node_to_regmap(np->parent);
> -	else
> -		regmap = dev_get_regmap(dev->parent, NULL) ?: ERR_PTR(-ENODEV);
> +	} else {
> +		regmap = device_node_to_regmap(np);

I started digging in device_node_to_regmap() to try to find an error that
could be used to trigger if the failover to dev_get_regmap() should be
tried, instead of always doing the failover on error. I got lost fairly
quickly, but it seems device_node_to_regmap() can return -EDEFER_PROBE.
While I'm not certain that it is applicable, that case should probably
not fall back to dev_get_regmap()...

Are there other error cases that should prevent the failover? I would
guess that it's perhaps just a single error that should trigger trying
the failover path? But I don't know, and which error if that's the case?

How much badness can be caused if syscon_node_to_regmap() fails for some
random obscure reason and the failover path is taken inadvertently? It
certainly smells bad for -EDEFER_PROBE, but do you have any insight in
other cases?

And after getting to approx that point a while back, I had other things
to take care of, and this fell off the table. Sorry!

Cheers,
Peter

> +		if (IS_ERR(regmap))
> +			regmap = dev_get_regmap(dev->parent, NULL) ?: ERR_PTR(-ENODEV);
> +	}
>  	if (IS_ERR(regmap)) {
>  		ret = PTR_ERR(regmap);
>  		dev_err(dev, "failed to get regmap: %d\n", ret);