Quoting Balakrishna Godavarthi (2018-06-18 10:07:31)
> Hi Stephen,
>
> Please find my comments inline.
>
> On 2018-06-18 22:12, Stephen Boyd wrote:
> > Quoting Balakrishna Godavarthi (2018-06-15 23:27:18)
> >> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> >> index 28ae6a17a595..1961e313aae7 100644
> >> --- a/drivers/bluetooth/hci_qca.c
> >> +++ b/drivers/bluetooth/hci_qca.c
> >> @@ -1031,9 +1145,118 @@ static struct hci_uart_proto qca_proto = {
> >> .dequeue = qca_dequeue,
> >> };
> >>
> >> +static const struct qca_vreg_data qca_cherokee_data = {
> >> + .soc_type = QCA_WCN3990,
> >> + .vregs = (struct qca_vreg []) {
> >> + { "vddio", 1352000, 1352000, 0 },
> >> + { "vddxtal", 1904000, 2040000, 0 },
> >> + { "vddcore", 1800000, 1800000, 1 },
> >> + { "vddpa", 1304000, 1304000, 1 },
> >> + { "vddldo", 3000000, 3312000, 1 },
> >
> > Load of 0 and 1 seems sort of odd. Are these made up to indicate "some
> > load" vs. "no load"?
> >
>
> [Bala]: this value specifies the output load current required to turn on
> the wcn3990.
> in struct defined vddio\vddxtal are smps, with fixed load.
> regs from vddcore/vddpa/vddldo are programmable line regulators,
> in which we need to set the basic load.
I got that part, but a load of 1 still seems like a nonsensical value.
Is it to workaround some issue with the regulator driver?
It's also pretty weird to be setting regulator voltages here in the
driver. Typically that's configured on the board and then the DT has the
right voltage already. The only case when the voltage may need to be set
by the driver is if the hardware can dynamically scale the voltage for
things like DVFS. It looks like here that the voltage is always fixed,
so probably we don't need to have any sort of voltage setting in this
driver and can rely on the DTS file to get things right.
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
