lkml.org 
[lkml]   [2012]   [May]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] regulator: core: use correct device for device supply lookup
    On Saturday 19 May 2012 11:56 PM, Mark Brown wrote:
    > * PGP Signed by an unknown key
    >
    > On Sat, May 19, 2012 at 11:26:00PM +0530, Laxman Dewangan wrote:
    >
    >> At the time of registration, as becasue there is valid
    >> reg_desc->supply_name and hence it tries to lookup the entry for
    >> <name>-supply i.e. v2-supply in this case for getting regulator_dev.
    >> regulator_register() {
    >> static struct regulator_dev *regulator_dev_lookup(struct device *dev,
    >> const char *supply,
    >> int *ret)
    >> {
    >> /* first do a dt based lookup */
    >> ----> Checked here, dev is not null but dev->of_node is null.
    >> if (dev&& dev->of_node) {
    >>
    >> ------------>The issue is that I am not getting here as dev->node is
    >> null here.
    > But how is this related your patch? What your patch does is change
    > things so that instead of trying to look up the supply in the context of
    > whatever device was passed in by the driver we try to look it up in the
    > context of the class device we create. I can't think of any situation
    > where I'd expect that to make matters any better - the class device
    > should certainly never appear in the device tree and isn't going to have
    > a stable name for non-DT systems either.
    >
    > I'm just not seeing any problem in the core here. It sounds to me like
    > the problem might be either with the regulator driver doing something
    > odd with the struct device it specifies when registering the regulator
    > (though I'm guessing that it's the tps65910 which looks to be doing
    > something sensible currently) or the device tree for the board being
    > odd. Looking at the changes you posted to tps65910 I suspect the issue
    > is that you've changed the driver to pass in the platform device for the
    > regulators as their device rather than the I2C device but it's the I2C
    > device which appears in the device tree bindings.
    >
    My board dts file is
    pmu: tps65910@d2 {
    compatible = "ti,tps65910";
    reg = <0xd2>;
    interrupt-parent = <&intc>;
    interrupts = < 0 118 0x04 >;

    #gpio-cells = <2>;
    gpio-controller;

    #interrupt-cells = <2>;
    interrupt-controller;

    regulators {
    vdd1_reg: vdd1 {
    regulator-min-microvolt = < 600000>;
    regulator-max-microvolt = <1500000>;
    regulator-always-on;
    regulator-boot-on;
    ti,regulator-ext-sleep-control = <0>;
    };
    vdd2_reg: vdd2 {
    regulator-min-microvolt = < 600000>;
    regulator-max-microvolt = <1500000>;
    regulator-always-on;
    regulator-boot-on;
    ti,regulator-ext-sleep-control = <4>;
    };
    };
    };

    So currently, when regulator_register gets called in
    tps65910-regulator.c, it sets the
    config.dev = tps65910->dev;
    config.of_node as
    config.of_node =
    of_find_node_by_name(tps65910->dev->of_node,
    info->name);
    So here config.of_node always shows NULL as the in
    tps65910->dev->of_node does not have name same as info->name ie.
    regulator name like vdd1, vdd2.

    So I changed it to pass the node containing"regulators" for
    of_find_node_by_name() and then it got proper of_node like vdd1_reg or
    vdd2_reg as per info_name.

    Now this node for vdd1_reg, vdd2_reg, is being passed to
    regulator_register. And for lookup of node, we should use the
    config.of_node which is set.

    If we still want to use the parent device for lookup then other way to
    use the config.of_node is to take the parameter of device node in this api.

    For this it is require to change the apis as

    static struct regulator_dev *regulator_dev_lookup(struct device *dev,
    const char *supply,
    int *ret)

    to
    static struct regulator_dev *regulator_dev_lookup(struct device *dev,
    struct device_node *dev_node,
    const char *supply,
    int *ret)


    and at the time of regulator registration, we should pass config->of_node.
    In this way we can still pass parent device and of_node which is passed
    by regulator driver.
    if (supply) {
    struct regulator_dev *r;

    r = regulator_dev_lookup(dev, config->of_node, supply,
    &ret);


    > If there is a change needed in the core you need to explain what you
    > believe that change will do.
    >
    I though this is straight but seems it is becoming more complex now.
    I will describe all this details if we agree to change require.



    \
     
     \ /
      Last update: 2012-05-19 21:41    [W:0.032 / U:0.108 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site