Re: [U-Boot] [PATCH 2/9] rtc: mc146818: Use probe() to set up the device

[Simon Glass]

On 18 October 2015 at 20:41, Bin Meng  wrote:
> Hi Simon,
>
> On Mon, Oct 19, 2015 at 10:38 AM, Simon Glass  wrote:
>> Hi Bin,
>>
>> On 18 October 2015 at 20:32, Bin Meng  wrote:
>>> Hi Simon,
>>>
>>> On Mon, Oct 19, 2015 at 10:26 AM, Simon Glass  wrote:
 Hi Bin,

 On 18 October 2015 at 20:22, Bin Meng  wrote:
> Hi Simon,
>
> On Mon, Oct 19, 2015 at 5:55 AM, Simon Glass  wrote:
>> At present this driver uses bind() to set up the device. The bind() 
>> method
>> should not touch the hardware, so move the init code to probe().
>>
>
> I think RTC should be initialized anyway. If moving it to probe, it
> may not be initialized by U-Boot before jumping to kernel.

 That's fine, but the correct way to do this is to probe the device,
 not put the init code into the bind() method.
>>>
>>> Yes, I agree. So maybe we explicitly trigger the probe somewhere in
>>> the initialization path?
>>>
>>
>> Or maybe just before jumping to the OS - e.g. in boot_prep_linux()?
>>
>
> Yes, that sounds good. And we may need consider other OSes as well.
> Maybe we need some consolidation.
>
>> [snip]
>>
>
> Regards,
> Bin

Applied to u-boot-x86.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 2/9] rtc: mc146818: Use probe() to set up the device

[Simon Glass]

At present this driver uses bind() to set up the device. The bind() method
should not touch the hardware, so move the init code to probe().

Signed-off-by: Simon Glass 
---

 drivers/rtc/mc146818.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/mc146818.c b/drivers/rtc/mc146818.c
index 9e94a80..da804d5 100644
--- a/drivers/rtc/mc146818.c
+++ b/drivers/rtc/mc146818.c
@@ -225,7 +225,7 @@ static int rtc_mc146818_write8(struct udevice *dev, 
unsigned int reg, int val)
return 0;
 }
 
-static int rtc_mc146818_bind(struct udevice *dev)
+static int rtc_mc146818_probe(struct udevice *dev)
 {
mc146818_init();
 
@@ -249,7 +249,7 @@ U_BOOT_DRIVER(rtc_mc146818) = {
.name = "rtc_mc146818",
.id = UCLASS_RTC,
.of_match = rtc_mc146818_ids,
-   .bind = rtc_mc146818_bind,
+   .probe = rtc_mc146818_probe,
.ops = _mc146818_ops,
 };
 
-- 
2.6.0.rc2.230.g3dd15c0

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 2/9] rtc: mc146818: Use probe() to set up the device

[Simon Glass]

Hi Bin,

On 18 October 2015 at 20:22, Bin Meng  wrote:
> Hi Simon,
>
> On Mon, Oct 19, 2015 at 5:55 AM, Simon Glass  wrote:
>> At present this driver uses bind() to set up the device. The bind() method
>> should not touch the hardware, so move the init code to probe().
>>
>
> I think RTC should be initialized anyway. If moving it to probe, it
> may not be initialized by U-Boot before jumping to kernel.

That's fine, but the correct way to do this is to probe the device,
not put the init code into the bind() method.

>
>> Signed-off-by: Simon Glass 
>> ---
>>
>>  drivers/rtc/mc146818.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/rtc/mc146818.c b/drivers/rtc/mc146818.c
>> index 9e94a80..da804d5 100644
>> --- a/drivers/rtc/mc146818.c
>> +++ b/drivers/rtc/mc146818.c
>> @@ -225,7 +225,7 @@ static int rtc_mc146818_write8(struct udevice *dev, 
>> unsigned int reg, int val)
>> return 0;
>>  }
>>
>> -static int rtc_mc146818_bind(struct udevice *dev)
>> +static int rtc_mc146818_probe(struct udevice *dev)
>>  {
>> mc146818_init();
>>
>> @@ -249,7 +249,7 @@ U_BOOT_DRIVER(rtc_mc146818) = {
>> .name = "rtc_mc146818",
>> .id = UCLASS_RTC,
>> .of_match = rtc_mc146818_ids,
>> -   .bind = rtc_mc146818_bind,
>> +   .probe = rtc_mc146818_probe,
>> .ops = _mc146818_ops,
>>  };
>>
>> --
>
> Regards,
> Bin

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 2/9] rtc: mc146818: Use probe() to set up the device

[Bin Meng]

Hi Simon,

On Mon, Oct 19, 2015 at 10:26 AM, Simon Glass  wrote:
> Hi Bin,
>
> On 18 October 2015 at 20:22, Bin Meng  wrote:
>> Hi Simon,
>>
>> On Mon, Oct 19, 2015 at 5:55 AM, Simon Glass  wrote:
>>> At present this driver uses bind() to set up the device. The bind() method
>>> should not touch the hardware, so move the init code to probe().
>>>
>>
>> I think RTC should be initialized anyway. If moving it to probe, it
>> may not be initialized by U-Boot before jumping to kernel.
>
> That's fine, but the correct way to do this is to probe the device,
> not put the init code into the bind() method.

Yes, I agree. So maybe we explicitly trigger the probe somewhere in
the initialization path?

>
>>
>>> Signed-off-by: Simon Glass 
>>> ---
>>>
>>>  drivers/rtc/mc146818.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/rtc/mc146818.c b/drivers/rtc/mc146818.c
>>> index 9e94a80..da804d5 100644
>>> --- a/drivers/rtc/mc146818.c
>>> +++ b/drivers/rtc/mc146818.c
>>> @@ -225,7 +225,7 @@ static int rtc_mc146818_write8(struct udevice *dev, 
>>> unsigned int reg, int val)
>>> return 0;
>>>  }
>>>
>>> -static int rtc_mc146818_bind(struct udevice *dev)
>>> +static int rtc_mc146818_probe(struct udevice *dev)
>>>  {
>>> mc146818_init();
>>>
>>> @@ -249,7 +249,7 @@ U_BOOT_DRIVER(rtc_mc146818) = {
>>> .name = "rtc_mc146818",
>>> .id = UCLASS_RTC,
>>> .of_match = rtc_mc146818_ids,
>>> -   .bind = rtc_mc146818_bind,
>>> +   .probe = rtc_mc146818_probe,
>>> .ops = _mc146818_ops,
>>>  };
>>>
>>> --
>>

Regards,
Bin
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 2/9] rtc: mc146818: Use probe() to set up the device

[Bin Meng]

Hi Simon,

On Mon, Oct 19, 2015 at 10:38 AM, Simon Glass  wrote:
> Hi Bin,
>
> On 18 October 2015 at 20:32, Bin Meng  wrote:
>> Hi Simon,
>>
>> On Mon, Oct 19, 2015 at 10:26 AM, Simon Glass  wrote:
>>> Hi Bin,
>>>
>>> On 18 October 2015 at 20:22, Bin Meng  wrote:
 Hi Simon,

 On Mon, Oct 19, 2015 at 5:55 AM, Simon Glass  wrote:
> At present this driver uses bind() to set up the device. The bind() method
> should not touch the hardware, so move the init code to probe().
>

 I think RTC should be initialized anyway. If moving it to probe, it
 may not be initialized by U-Boot before jumping to kernel.
>>>
>>> That's fine, but the correct way to do this is to probe the device,
>>> not put the init code into the bind() method.
>>
>> Yes, I agree. So maybe we explicitly trigger the probe somewhere in
>> the initialization path?
>>
>
> Or maybe just before jumping to the OS - e.g. in boot_prep_linux()?
>

Yes, that sounds good. And we may need consider other OSes as well.
Maybe we need some consolidation.

> [snip]
>

Regards,
Bin
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 2/9] rtc: mc146818: Use probe() to set up the device

[Bin Meng]

Hi Simon,

On Mon, Oct 19, 2015 at 5:55 AM, Simon Glass  wrote:
> At present this driver uses bind() to set up the device. The bind() method
> should not touch the hardware, so move the init code to probe().
>

I think RTC should be initialized anyway. If moving it to probe, it
may not be initialized by U-Boot before jumping to kernel.

> Signed-off-by: Simon Glass 
> ---
>
>  drivers/rtc/mc146818.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/rtc/mc146818.c b/drivers/rtc/mc146818.c
> index 9e94a80..da804d5 100644
> --- a/drivers/rtc/mc146818.c
> +++ b/drivers/rtc/mc146818.c
> @@ -225,7 +225,7 @@ static int rtc_mc146818_write8(struct udevice *dev, 
> unsigned int reg, int val)
> return 0;
>  }
>
> -static int rtc_mc146818_bind(struct udevice *dev)
> +static int rtc_mc146818_probe(struct udevice *dev)
>  {
> mc146818_init();
>
> @@ -249,7 +249,7 @@ U_BOOT_DRIVER(rtc_mc146818) = {
> .name = "rtc_mc146818",
> .id = UCLASS_RTC,
> .of_match = rtc_mc146818_ids,
> -   .bind = rtc_mc146818_bind,
> +   .probe = rtc_mc146818_probe,
> .ops = _mc146818_ops,
>  };
>
> --

Regards,
Bin
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 2/9] rtc: mc146818: Use probe() to set up the device

[Simon Glass]

Hi Bin,

On 18 October 2015 at 20:32, Bin Meng  wrote:
> Hi Simon,
>
> On Mon, Oct 19, 2015 at 10:26 AM, Simon Glass  wrote:
>> Hi Bin,
>>
>> On 18 October 2015 at 20:22, Bin Meng  wrote:
>>> Hi Simon,
>>>
>>> On Mon, Oct 19, 2015 at 5:55 AM, Simon Glass  wrote:
 At present this driver uses bind() to set up the device. The bind() method
 should not touch the hardware, so move the init code to probe().

>>>
>>> I think RTC should be initialized anyway. If moving it to probe, it
>>> may not be initialized by U-Boot before jumping to kernel.
>>
>> That's fine, but the correct way to do this is to probe the device,
>> not put the init code into the bind() method.
>
> Yes, I agree. So maybe we explicitly trigger the probe somewhere in
> the initialization path?
>

Or maybe just before jumping to the OS - e.g. in boot_prep_linux()?

[snip]

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot