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 X-Spam-Level: X-Spam-Status: No, score=-10.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 54154C4338F for ; Fri, 20 Aug 2021 08:10:46 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3CF8361042 for ; Fri, 20 Aug 2021 08:10:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231665AbhHTILU (ORCPT ); Fri, 20 Aug 2021 04:11:20 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60182 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230195AbhHTILS (ORCPT ); Fri, 20 Aug 2021 04:11:18 -0400 Received: from mail-qt1-x831.google.com (mail-qt1-x831.google.com [IPv6:2607:f8b0:4864:20::831]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3A068C06175F for ; Fri, 20 Aug 2021 01:10:41 -0700 (PDT) Received: by mail-qt1-x831.google.com with SMTP id d9so6813066qty.12 for ; Fri, 20 Aug 2021 01:10:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=g5IS6e3Hnyb2jRAb6f1KMnSZamwQJcSDoTsblYNlRlY=; b=AZ9hPtV94XSFU7TEKvhcU+et+rJ+GpExWH9lrUhoqWoy0ewHu2M34a/uO6l9DjkV3W f0oJk7nxCalOBi4OJVItLOhpuaLrPNMPtOlhahFsCqEnDmrPKqvI4ExcusqN0kmf2dmm X6aVP3hh5uVmOusZVgIhxqBCEvpuDtGXIU0bCJxnBEYFye8qFaFZb0rRPDeTD4tGclZw r6C1JJE68Lj5FmT8KeHnhA7VE0yN5hMhHlb84zbNqFmbhPEbOEPv33O68IOJr/4+cmXs 8UuTDZzFcGnR83wrcms+xf3UQTABzatiaTOhd2YkS23GfoR9Q7lX2ajxwnwJ0ubC8bl1 XkYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=g5IS6e3Hnyb2jRAb6f1KMnSZamwQJcSDoTsblYNlRlY=; b=etgr1PdDqrQSx2ffotXI71Qaj7C6PvGhFQucspBzF8gVsH+pPEEnOc6qhyRVnDpOZl F0rRd68QruFZDYNx7+7zs+hQg5s6OYfE+TtGl7SnXC0pJoduqqR5zKiYcXdfN52xok/+ kd3E4orS9Dp/uD8O7I3mIUpXw8AhskBWRKcIaN6hmxjckCMQrqAfCj6G6ibgFFNdgjFs LunLiYIS1dCzv2Oc7WC8uZIU/1lQOpsKKfu2xiehKnYX9pyVw2SIr1EEG+PEfRG6P+Oz wnjaCSzRiCVV3WAQeltk4VolSUrZRAIVKwA9XnMrRwQa7bJ/3HScDG2jeW1Yf7pRKOuR 2dvw== X-Gm-Message-State: AOAM532vEuSSXvusP5TiLrkMkeA0gyIZkKxaO34yMl6xDkQj6I9iv/cK CJUW4h6uZSLSS83WxKx6EH51YT+XOhjZLt3BuvWcQg== X-Google-Smtp-Source: ABdhPJykvHLGPJLrte3XmQ+2fZnjn7MpEKijpvU45xzTfUuHaBNaF5vvWd95j2y77Xi7ngLBiT0SX8r507FJf1UymdA= X-Received: by 2002:a05:622a:13c8:: with SMTP id p8mr16634457qtk.238.1629447040101; Fri, 20 Aug 2021 01:10:40 -0700 (PDT) MIME-Version: 1.0 References: <20210817005507.1507580-1-dmitry.baryshkov@linaro.org> <20210817005507.1507580-11-dmitry.baryshkov@linaro.org> In-Reply-To: From: Dmitry Baryshkov Date: Fri, 20 Aug 2021 11:10:29 +0300 Message-ID: Subject: Re: [RFC PATCH 10/15] pwrseq: add support for QCA BT+WiFi power sequencer To: Bjorn Andersson Cc: Andy Gross , Ulf Hansson , Marcel Holtmann , Johan Hedberg , Luiz Augusto von Dentz , Kalle Valo , "David S. Miller" , Jakub Kicinski , Stanimir Varbanov , "open list:DRM DRIVER FOR MSM ADRENO GPU" , linux-mmc@vger.kernel.org, open list , linux-bluetooth@vger.kernel.org, ath10k@lists.infradead.org, linux-wireless@vger.kernel.org, netdev@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On Fri, 20 Aug 2021 at 02:17, Bjorn Andersson wrote: > > On Mon 16 Aug 17:55 PDT 2021, Dmitry Baryshkov wrote: > [..] > > diff --git a/drivers/power/pwrseq/pwrseq_qca.c b/drivers/power/pwrseq/pwrseq_qca.c > > new file mode 100644 > > index 000000000000..3421a4821126 > > --- /dev/null > > +++ b/drivers/power/pwrseq/pwrseq_qca.c > > @@ -0,0 +1,290 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Copyright (c) 2021, Linaro Ltd. > > + * > > + * Author: Dmitry Baryshkov > > + * > > + * Power Sequencer for Qualcomm WiFi + BT SoCs > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +/* > > + * Voltage regulator information required for configuring the > > + * QCA WiFi+Bluetooth chipset > > + */ > > +struct qca_vreg { > > + const char *name; > > + unsigned int load_uA; > > +}; > > + > > +struct qca_device_data { > > + struct qca_vreg vddio; > > Any particular reason why this isn't just the first entry in vregs and > operated as part of the bulk API? Because VDDIO should be up before bringing the rest of the power sources (at least for wcn39xx). This is usually the case since VDDIO is S4A, but I'd still prefer to express this in the code. And register_bulk_enable powers up all the supplies asynchronously, thus it can not guarantee that the first entry would be powered up first. > > > + struct qca_vreg *vregs; > > + size_t num_vregs; > > + bool has_bt_en; > > + bool has_wifi_en; > > +}; > > + > > +struct pwrseq_qca; > > +struct pwrseq_qca_one { > > + struct pwrseq_qca *common; > > + struct gpio_desc *enable; > > +}; > > + > > +#define PWRSEQ_QCA_WIFI 0 > > +#define PWRSEQ_QCA_BT 1 > > + > > +#define PWRSEQ_QCA_MAX 2 > > + > > +struct pwrseq_qca { > > + struct regulator *vddio; > > + struct gpio_desc *sw_ctrl; > > + struct pwrseq_qca_one pwrseq_qcas[PWRSEQ_QCA_MAX]; > > + int num_vregs; > > + struct regulator_bulk_data vregs[]; > > +}; > > + > > +static int pwrseq_qca_power_on(struct pwrseq *pwrseq) > > +{ > > + struct pwrseq_qca_one *qca_one = pwrseq_get_data(pwrseq); > > + int ret; > > + > > + if (qca_one->common->vddio) { > > devm_regulator_get() doesn't return NULL, so this is always true. This is more of the safety guard for the cases when the qca doesn't have the special vddio supply. > > > + ret = regulator_enable(qca_one->common->vddio); > > + if (ret) > > + return ret; > > + } > > + > > + ret = regulator_bulk_enable(qca_one->common->num_vregs, qca_one->common->vregs); > > + if (ret) > > + goto vddio_off; > > + > > + if (qca_one->enable) { > > + gpiod_set_value_cansleep(qca_one->enable, 0); > > + msleep(50); > > + gpiod_set_value_cansleep(qca_one->enable, 1); > > + msleep(150); > > + } > > + > > + if (qca_one->common->sw_ctrl) { > > + bool sw_ctrl_state = gpiod_get_value_cansleep(qca_one->common->sw_ctrl); > > + dev_dbg(&pwrseq->dev, "SW_CTRL is %d", sw_ctrl_state); > > + } > > + > > + return 0; > > + > > +vddio_off: > > + regulator_disable(qca_one->common->vddio); > > + > > + return ret; > > +} > [..] > > +static int pwrseq_qca_probe(struct platform_device *pdev) > > +{ > > + struct pwrseq_qca *pwrseq_qca; > > + struct pwrseq *pwrseq; > > + struct pwrseq_provider *provider; > > + struct device *dev = &pdev->dev; > > + struct pwrseq_onecell_data *onecell; > > + const struct qca_device_data *data; > > + int ret, i; > > + > > + data = device_get_match_data(dev); > > + if (!data) > > + return -EINVAL; > > + > > + pwrseq_qca = devm_kzalloc(dev, struct_size(pwrseq_qca, vregs, data->num_vregs), GFP_KERNEL); > > + if (!pwrseq_qca) > > + return -ENOMEM; > > + > > + onecell = devm_kzalloc(dev, struct_size(onecell, pwrseqs, PWRSEQ_QCA_MAX), GFP_KERNEL); > > + if (!onecell) > > + return -ENOMEM; > > + > > + ret = pwrseq_qca_regulators_init(dev, pwrseq_qca, data); > > + if (ret) > > + return ret; > > + > > + if (data->has_wifi_en) { > > + pwrseq_qca->pwrseq_qcas[PWRSEQ_QCA_WIFI].enable = devm_gpiod_get(dev, "wifi-enable", GPIOD_OUT_LOW); > > + if (IS_ERR(pwrseq_qca->pwrseq_qcas[PWRSEQ_QCA_WIFI].enable)) { > > + return dev_err_probe(dev, PTR_ERR(pwrseq_qca->pwrseq_qcas[PWRSEQ_QCA_WIFI].enable), > > + "failed to acquire WIFI enable GPIO\n"); > > + } > > + } > > + > > + if (data->has_bt_en) { > > + pwrseq_qca->pwrseq_qcas[PWRSEQ_QCA_BT].enable = devm_gpiod_get(dev, "bt-enable", GPIOD_OUT_LOW); > > + if (IS_ERR(pwrseq_qca->pwrseq_qcas[PWRSEQ_QCA_BT].enable)) { > > + return dev_err_probe(dev, PTR_ERR(pwrseq_qca->pwrseq_qcas[PWRSEQ_QCA_BT].enable), > > + "failed to acquire BT enable GPIO\n"); > > + } > > + } > > + > > + pwrseq_qca->sw_ctrl = devm_gpiod_get_optional(dev, "swctrl", GPIOD_IN); > > + if (IS_ERR(pwrseq_qca->sw_ctrl)) { > > + return dev_err_probe(dev, PTR_ERR(pwrseq_qca->sw_ctrl), > > + "failed to acquire SW_CTRL gpio\n"); > > + } else if (!pwrseq_qca->sw_ctrl) > > + dev_info(dev, "No SW_CTRL gpio\n"); > > Some {} around the else as well please. ack > > Regards, > Bjorn -- With best wishes Dmitry