Re: [PATCH 2/2] clk: clk-sccg-pll: Drop prepare/unprepare for SCCG_PLL2
On Wed, Aug 22, 2018 at 06:07:01PM -0700, Andrey Smirnov wrote:
> On Wed, Aug 22, 2018 at 12:31 AM Sascha Hauer wrote:
> >
> > On Mon, Aug 20, 2018 at 11:28:53PM -0700, Andrey Smirnov wrote:
> > > A number of PLL pairs (e.g. "sys1_pll1" and "sys1_pll2") share the
> > > same configuration register, so touching PD bit, as is done for
> > > SCCG_PLL2 in its prepare/unprepare methods will result in shut down of
> > > both PLLs. This is very undesireable, since attempting to re-parent a
> > > clock to "sys1_pll2" might result in complete system shutdown due to
> > > "sys1_pll1" being shut-down as a part of re-parenting process.
> >
> > I can imagine that there are problems with the way it is currently
> > handled, but the scenario you describe shouldn't happen. "sys1_pll1"
> > will never be shut down because it doesn't have a disable hook:
> >
> > static const struct clk_ops clk_sccg_pll1_ops = {
> > .is_enabled = clk_pll1_is_prepared,
> > .recalc_rate= clk_pll1_recalc_rate,
> > .round_rate = clk_pll1_round_rate,
> > .set_rate = clk_pll1_set_rate,
> > };
> >
> > static const struct clk_ops clk_sccg_pll2_ops = {
> > .enable = clk_pll1_prepare,
> > .disable= clk_pll1_unprepare,
> > .recalc_rate= clk_pll2_recalc_rate,
> > .round_rate = clk_pll2_round_rate,
> > .set_rate = clk_pll2_set_rate,
> > };
> >
> > Have I missed something?
> >
>
> Yes, but that's probably due to my explanation being too vague, so let
> me try again. Both clocks "sys1_pll1" and "sys1_pll2" are
> controlled/configured via separate bit fields in the _same_ register.
> That register also has a single "powerdown"/"PD" bit that is shared
> between both PLLs and affects them both. The way code is currently
> implemented calling clk_disable() on "sys1_pll2" will clear PD bit and
> as a result "sys1_pll1" will be shut down as well. This is not that
> far-fetched of a scenario because a number of derived clocks that go
> out to individual peripherals are specified with
> CLK_OPS_PARENT_ENABLE, so trivial clock re-parenting like this one:
> https://github.com/ndreys/barebox/commit/802d1d5be1f1f3df996ba26b115dd3a55bc10c67
> will result in hung system.
>
> Hopefully this does a better job of explaining what I was trying to fix.
Yeah, I thought you mixed up both PLLs, but reading your initial
eplanation again you were right from the start.
I agree that we should just remove the enable/disable hooks and see what
we have to do when somebody actually tries to do something with the
clocks (other than get the rate from)
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 19/22] bbu: Remove logical negation in barebox_update_handler_exists()
On Wed, Aug 22, 2018 at 11:42 PM Sascha Hauer wrote:
>
> On Wed, Aug 22, 2018 at 05:01:41PM -0700, Andrey Smirnov wrote:
> > On Wed, Aug 22, 2018 at 12:09 AM Sascha Hauer
> > wrote:
> > >
> > > On Mon, Aug 20, 2018 at 11:26:00PM -0700, Andrey Smirnov wrote:
> > > > Returning !bbu_find_handler() from barebox_update_handler_exists()
> > > > would return the opposite result from what the name of that funciton
> > > > implies. Drop the "!" to make it behave as expected.
> > > >
> > > > Signed-off-by: Andrey Smirnov
> > > > ---
> > > > common/bbu.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/common/bbu.c b/common/bbu.c
> > > > index 11e44f4a7..69ccac68a 100644
> > > > --- a/common/bbu.c
> > > > +++ b/common/bbu.c
> > > > @@ -151,7 +151,7 @@ bool barebox_update_handler_exists(struct bbu_data
> > > > *data)
> > > > if (!data->handler_name)
> > > > return false;
> > > >
> > > > - return !bbu_find_handler(data->handler_name);
> > > > + return bbu_find_handler(data->handler_name);
> > >
> > > As bbu_find_handler() returns a pointer maybe better '!!' or
> > > bbu_find_handler() != NULL?
> > >
> >
> > That shouldn't be necessary since barebox_update_handler_exists()
> > returns bool(real type name is _Bool) which explicitly specifies that
> > a cast of any scalar value to it would be normalized to 1 or 0 (as per
> > C99 standard from whence it came). Otherwise you'd be able to end up
> > in a situation where bool1 && bool2 && (bool1 != bool2) evaluates to
> > true.
> >
> > To give you more concrete example, here's what the last portion of
> > that function compiles to on AArch64:
> >
> > 2924: 975e bl 269c
> > 2928: f11f cmp x0, #0x0
> > 292c: 1a9f07e0 cset w0, ne // ne = any
> > 2930: f9400bf3 ldr x19, [sp, #16]
> > 2934: a8c27bfd ldp x29, x30, [sp], #32
> > 2938: d65f03c0 ret
> > 293c: 52800020 mov w0, #0x1// #1
> > 2940: 17fc b 2930
> > 2944: 5280 mov w0, #0x0// #0
> > 2948: 17fa b 2930
> >
> > and on AArch32 (Thumb):
> >
> > 18e0: f7ff ff48 bl 1774
> > 18e4: 3000 adds r0, #0
> > 18e6: bf18 it ne
> > 18e8: 2001 movne r0, #1
> > 18ea: bd10 pop {r4, pc}
> > 18ec: 2001 movs r0, #1
> > 18ee: e7fc b.n 18ea
> >
> > as you can see both cases already have code to explicitly convert the
> > result of the function to 0/1.
> >
> > I am more than happy to add ether !! or != NULL if you still think
> > that'd be better, it just I don't think it will have any practical
> > effect.
>
> For sure it doesn't have a practical effect, it's merely a sign to show
> "I know I'm casting a pointer to bool". Probably I'm just used to add
> an explicit cast there and it's just a matter of taste.
>
Since I have two people in favor of adding a != NULL there, I'll just
add it in v2 since I don't have a very strong opinion on the subject.
Thanks,
Andrey Smirnov
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 04/22] ARM: i.MX: bbu: Move inner-image type check
On Wed, Aug 22, 2018 at 05:06:55PM -0700, Andrey Smirnov wrote: > On Tue, Aug 21, 2018 at 11:52 PM Sascha Hauer wrote: > > > @@ -137,7 +165,8 @@ static int imx_bbu_internal_v1_update(struct > > > bbu_handler *handler, struct bbu_da > > > container_of(handler, struct imx_internal_bbu_handler, > > > handler); > > > int ret; > > > > > > - ret = imx_bbu_check_prereq(data->devicefile, data); > > > + ret = imx_bbu_check_prereq(imx_handler, data->devicefile, data, > > > +filetype_unknown); > > > > Why filetype_unknown here? in the v2 version we have > > filetype_imx_image_v2. I would expect filetype_imx_image_v1 here. > > > > Purely because original code didn't do any type checking of "inner" > image, so I specified filetype_unknown and and added special handling > for it to preserve the status quo. It sounds like you think that it > would be better to change the original behavior such that there _is_ > an "inner" image type check for v1 one header. That would be my > preference as well, since that'll allow me to get rid of special > filetype_unknown case, so that's what I'll do in v2. Not sure why the v1 code is different here, it probably just grew like that and I never noticed. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 19/22] bbu: Remove logical negation in barebox_update_handler_exists()
On Wed, Aug 22, 2018 at 05:01:41PM -0700, Andrey Smirnov wrote:
> On Wed, Aug 22, 2018 at 12:09 AM Sascha Hauer wrote:
> >
> > On Mon, Aug 20, 2018 at 11:26:00PM -0700, Andrey Smirnov wrote:
> > > Returning !bbu_find_handler() from barebox_update_handler_exists()
> > > would return the opposite result from what the name of that funciton
> > > implies. Drop the "!" to make it behave as expected.
> > >
> > > Signed-off-by: Andrey Smirnov
> > > ---
> > > common/bbu.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/common/bbu.c b/common/bbu.c
> > > index 11e44f4a7..69ccac68a 100644
> > > --- a/common/bbu.c
> > > +++ b/common/bbu.c
> > > @@ -151,7 +151,7 @@ bool barebox_update_handler_exists(struct bbu_data
> > > *data)
> > > if (!data->handler_name)
> > > return false;
> > >
> > > - return !bbu_find_handler(data->handler_name);
> > > + return bbu_find_handler(data->handler_name);
> >
> > As bbu_find_handler() returns a pointer maybe better '!!' or
> > bbu_find_handler() != NULL?
> >
>
> That shouldn't be necessary since barebox_update_handler_exists()
> returns bool(real type name is _Bool) which explicitly specifies that
> a cast of any scalar value to it would be normalized to 1 or 0 (as per
> C99 standard from whence it came). Otherwise you'd be able to end up
> in a situation where bool1 && bool2 && (bool1 != bool2) evaluates to
> true.
>
> To give you more concrete example, here's what the last portion of
> that function compiles to on AArch64:
>
> 2924: 975e bl 269c
> 2928: f11f cmp x0, #0x0
> 292c: 1a9f07e0 cset w0, ne // ne = any
> 2930: f9400bf3 ldr x19, [sp, #16]
> 2934: a8c27bfd ldp x29, x30, [sp], #32
> 2938: d65f03c0 ret
> 293c: 52800020 mov w0, #0x1// #1
> 2940: 17fc b 2930
> 2944: 5280 mov w0, #0x0// #0
> 2948: 17fa b 2930
>
> and on AArch32 (Thumb):
>
> 18e0: f7ff ff48 bl 1774
> 18e4: 3000 adds r0, #0
> 18e6: bf18 it ne
> 18e8: 2001 movne r0, #1
> 18ea: bd10 pop {r4, pc}
> 18ec: 2001 movs r0, #1
> 18ee: e7fc b.n 18ea
>
> as you can see both cases already have code to explicitly convert the
> result of the function to 0/1.
>
> I am more than happy to add ether !! or != NULL if you still think
> that'd be better, it just I don't think it will have any practical
> effect.
For sure it doesn't have a practical effect, it's merely a sign to show
"I know I'm casting a pointer to bool". Probably I'm just used to add
an explicit cast there and it's just a matter of taste.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 19/22] bbu: Remove logical negation in barebox_update_handler_exists()
Hi Andrey. > > > > > > - return !bbu_find_handler(data->handler_name); > > > + return bbu_find_handler(data->handler_name); > > > > As bbu_find_handler() returns a pointer maybe better '!!' or > > bbu_find_handler() != NULL? > > > > That shouldn't be necessary At least to me the xxx != NULL would express the intent in a more clear way. But then if this becomes a common pattern then I will also learn that. But you got my preference. Sam ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 2/2] clk: clk-sccg-pll: Drop prepare/unprepare for SCCG_PLL2
On Wed, Aug 22, 2018 at 12:31 AM Sascha Hauer wrote:
>
> On Mon, Aug 20, 2018 at 11:28:53PM -0700, Andrey Smirnov wrote:
> > A number of PLL pairs (e.g. "sys1_pll1" and "sys1_pll2") share the
> > same configuration register, so touching PD bit, as is done for
> > SCCG_PLL2 in its prepare/unprepare methods will result in shut down of
> > both PLLs. This is very undesireable, since attempting to re-parent a
> > clock to "sys1_pll2" might result in complete system shutdown due to
> > "sys1_pll1" being shut-down as a part of re-parenting process.
>
> I can imagine that there are problems with the way it is currently
> handled, but the scenario you describe shouldn't happen. "sys1_pll1"
> will never be shut down because it doesn't have a disable hook:
>
> static const struct clk_ops clk_sccg_pll1_ops = {
> .is_enabled = clk_pll1_is_prepared,
> .recalc_rate= clk_pll1_recalc_rate,
> .round_rate = clk_pll1_round_rate,
> .set_rate = clk_pll1_set_rate,
> };
>
> static const struct clk_ops clk_sccg_pll2_ops = {
> .enable = clk_pll1_prepare,
> .disable= clk_pll1_unprepare,
> .recalc_rate= clk_pll2_recalc_rate,
> .round_rate = clk_pll2_round_rate,
> .set_rate = clk_pll2_set_rate,
> };
>
> Have I missed something?
>
Yes, but that's probably due to my explanation being too vague, so let
me try again. Both clocks "sys1_pll1" and "sys1_pll2" are
controlled/configured via separate bit fields in the _same_ register.
That register also has a single "powerdown"/"PD" bit that is shared
between both PLLs and affects them both. The way code is currently
implemented calling clk_disable() on "sys1_pll2" will clear PD bit and
as a result "sys1_pll1" will be shut down as well. This is not that
far-fetched of a scenario because a number of derived clocks that go
out to individual peripherals are specified with
CLK_OPS_PARENT_ENABLE, so trivial clock re-parenting like this one:
https://github.com/ndreys/barebox/commit/802d1d5be1f1f3df996ba26b115dd3a55bc10c67
will result in hung system.
Hopefully this does a better job of explaining what I was trying to fix.
Thanks,
Andrey Smirnov
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 18/22] ARM: i.MX: bbu: Adjust error code check for pwrite()
On Wed, Aug 22, 2018 at 12:01 AM Sascha Hauer wrote:
>
> On Mon, Aug 20, 2018 at 11:25:59PM -0700, Andrey Smirnov wrote:
> > Pwrite() will return the amount bytes written or negative error code
> > on success, so we need to do two things with it:
> >
> > 1. Check it against "image_len" to make sure we actually wrote all
> >of the data
> >
> > 2. Set it to zero in case of success, since that is what code in
> >barebox_update() expects to happen
> >
> > Signed-off-by: Andrey Smirnov
> > ---
> > arch/arm/mach-imx/imx-bbu-internal.c | 11 +--
> > 1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/mach-imx/imx-bbu-internal.c
> > b/arch/arm/mach-imx/imx-bbu-internal.c
> > index d83eb972c..70af5ef84 100644
> > --- a/arch/arm/mach-imx/imx-bbu-internal.c
> > +++ b/arch/arm/mach-imx/imx-bbu-internal.c
> > @@ -86,6 +86,7 @@ static int imx_bbu_write_device(struct
> > imx_internal_bbu_handler *imx_handler,
> > const void *buf, int image_len)
> > {
> > int fd, ret, offset = 0;
> > + bool partial_write;
> >
> > fd = open(devicefile, O_RDWR | O_CREAT);
> > if (fd < 0)
> > @@ -117,8 +118,14 @@ static int imx_bbu_write_device(struct
> > imx_internal_bbu_handler *imx_handler,
> > }
> >
> > ret = pwrite(fd, buf, image_len, offset);
> > - if (ret < 0)
> > + partial_write = ret > 0 && ret != image_len;
> > + if (ret < 0 || partial_write) {
> > + ret = partial_write ? -EIO : ret;
> > +
> > + pr_err("writing to %s failed with %s\n", devicefile,
> > +strerror(-ret));
> > goto err_close;
>
> Do we need a pwrite_full analog to write_full?
>
Sure, why not? Should allow me to drop that partial_write variable.
Will do in v2.
Thanks,
Andrey Smirnov
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 04/22] ARM: i.MX: bbu: Move inner-image type check
On Tue, Aug 21, 2018 at 11:52 PM Sascha Hauer wrote:
>
> On Mon, Aug 20, 2018 at 11:25:45PM -0700, Andrey Smirnov wrote:
> > Since imx_bbu_check_prereq() already uses file_detect_type() and we've
> > extended it to understand i.MX boot image file type, we can simplify a
> > bunch of repetitive code as follows:
> >
> > 1. Convert all checks from IVT_BARKER to filetype_imx_image_v2
> >check
> >
> > 2. Move all of the checking to be a part of imx_bbu_check_prereq()
> >
> > Signed-off-by: Andrey Smirnov
> > ---
> > arch/arm/mach-imx/imx-bbu-internal.c | 64 +---
> > 1 file changed, 39 insertions(+), 25 deletions(-)
> >
> > diff --git a/arch/arm/mach-imx/imx-bbu-internal.c
> > b/arch/arm/mach-imx/imx-bbu-internal.c
> > index 67ae2961c..ea57b2772 100644
> > --- a/arch/arm/mach-imx/imx-bbu-internal.c
> > +++ b/arch/arm/mach-imx/imx-bbu-internal.c
> > @@ -106,11 +106,39 @@ err_close:
> > return ret;
> > }
> >
> > -static int imx_bbu_check_prereq(const char *devicefile, struct bbu_data
> > *data)
> > +static int imx_bbu_check_prereq(struct imx_internal_bbu_handler
> > *imx_handler,
> > + const char *devicefile, struct bbu_data *data,
> > + enum filetype expected_type)
> > {
> > int ret;
> > -
> > - if (file_detect_type(data->image, data->len) != filetype_arm_barebox)
> > {
> > + const void *blob;
> > + size_t len;
> > + enum filetype type;
> > +
> > + type = file_detect_type(data->image, data->len);
> > +
> > + switch (type) {
> > + case filetype_arm_barebox:
> > + /*
> > + * Specifying expected_type as unknow will disable the
> > + * inner image type check
> > + */
> > + if (expected_type == filetype_unknown)
> > + break;
> > +
> > + blob = data->image + imx_handler->flash_header_offset;
> > + len = data->len - imx_handler->flash_header_offset;
> > + type = file_detect_type(blob, len);
> > +
> > + if (type != expected_type) {
> > + pr_err("Expected image type: %s, "
> > +"detected image type: %s\n",
> > +file_type_to_string(expected_type),
> > +file_type_to_string(type));
> > + return -EINVAL;
> > + }
> > + break;
> > + default:
> > if (!bbu_force(data, "Not an ARM barebox image"))
> > return -EINVAL;
> > }
> > @@ -137,7 +165,8 @@ static int imx_bbu_internal_v1_update(struct
> > bbu_handler *handler, struct bbu_da
> > container_of(handler, struct imx_internal_bbu_handler,
> > handler);
> > int ret;
> >
> > - ret = imx_bbu_check_prereq(data->devicefile, data);
> > + ret = imx_bbu_check_prereq(imx_handler, data->devicefile, data,
> > +filetype_unknown);
>
> Why filetype_unknown here? in the v2 version we have
> filetype_imx_image_v2. I would expect filetype_imx_image_v1 here.
>
Purely because original code didn't do any type checking of "inner"
image, so I specified filetype_unknown and and added special handling
for it to preserve the status quo. It sounds like you think that it
would be better to change the original behavior such that there _is_
an "inner" image type check for v1 one header. That would be my
preference as well, since that'll allow me to get rid of special
filetype_unknown case, so that's what I'll do in v2.
Thanks,
Andrey Smirnov
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 06/22] ARM: i.MX: bbu: Consolidate vairous update helpers
On Tue, Aug 21, 2018 at 11:52 PM Sascha Hauer wrote: > > In The subject: s/vairous/various/ > Will fix in v2. Thanks, Andrey Smirnov ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 19/22] bbu: Remove logical negation in barebox_update_handler_exists()
On Wed, Aug 22, 2018 at 12:09 AM Sascha Hauer wrote:
>
> On Mon, Aug 20, 2018 at 11:26:00PM -0700, Andrey Smirnov wrote:
> > Returning !bbu_find_handler() from barebox_update_handler_exists()
> > would return the opposite result from what the name of that funciton
> > implies. Drop the "!" to make it behave as expected.
> >
> > Signed-off-by: Andrey Smirnov
> > ---
> > common/bbu.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/common/bbu.c b/common/bbu.c
> > index 11e44f4a7..69ccac68a 100644
> > --- a/common/bbu.c
> > +++ b/common/bbu.c
> > @@ -151,7 +151,7 @@ bool barebox_update_handler_exists(struct bbu_data
> > *data)
> > if (!data->handler_name)
> > return false;
> >
> > - return !bbu_find_handler(data->handler_name);
> > + return bbu_find_handler(data->handler_name);
>
> As bbu_find_handler() returns a pointer maybe better '!!' or
> bbu_find_handler() != NULL?
>
That shouldn't be necessary since barebox_update_handler_exists()
returns bool(real type name is _Bool) which explicitly specifies that
a cast of any scalar value to it would be normalized to 1 or 0 (as per
C99 standard from whence it came). Otherwise you'd be able to end up
in a situation where bool1 && bool2 && (bool1 != bool2) evaluates to
true.
To give you more concrete example, here's what the last portion of
that function compiles to on AArch64:
2924: 975e bl 269c
2928: f11f cmp x0, #0x0
292c: 1a9f07e0 cset w0, ne // ne = any
2930: f9400bf3 ldr x19, [sp, #16]
2934: a8c27bfd ldp x29, x30, [sp], #32
2938: d65f03c0 ret
293c: 52800020 mov w0, #0x1// #1
2940: 17fc b 2930
2944: 5280 mov w0, #0x0// #0
2948: 17fa b 2930
and on AArch32 (Thumb):
18e0: f7ff ff48 bl 1774
18e4: 3000 adds r0, #0
18e6: bf18 it ne
18e8: 2001 movne r0, #1
18ea: bd10 pop {r4, pc}
18ec: 2001 movs r0, #1
18ee: e7fc b.n 18ea
as you can see both cases already have code to explicitly convert the
result of the function to 0/1.
I am more than happy to add ether !! or != NULL if you still think
that'd be better, it just I don't think it will have any practical
effect.
Thanks,
Andrey Smirnov
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
[PATCH v4 3/3] at91sam9263ek: add PHY, miitool etc. to config
Added Davicom phy to the config now it is supported. Add miitool to show the phy info. Also add a bunch of extra commands that can be useful when handling the evaluation board. Signed-off-by: Sam Ravnborg --- arch/arm/configs/at91sam9263ek_defconfig | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/arch/arm/configs/at91sam9263ek_defconfig b/arch/arm/configs/at91sam9263ek_defconfig index e8ad841fa..45c6f79de 100644 --- a/arch/arm/configs/at91sam9263ek_defconfig +++ b/arch/arm/configs/at91sam9263ek_defconfig @@ -18,7 +18,7 @@ CONFIG_BOOTM_OFTREE=y CONFIG_BOOTM_OFTREE_UIMAGE=y CONFIG_CONSOLE_ACTIVATE_ALL=y CONFIG_DEFAULT_ENVIRONMENT_GENERIC=y -# CONFIG_CMD_ARM_CPUINFO is not set +CONFIG_CMD_DMESG=y CONFIG_LONGHELP=y CONFIG_CMD_IOMEM=y CONFIG_CMD_MEMINFO=y @@ -33,22 +33,32 @@ CONFIG_CMD_PRINTENV=y CONFIG_CMD_SAVEENV=y CONFIG_CMD_SLEEP=y CONFIG_CMD_DHCP=y +CONFIG_CMD_HOST=y +CONFIG_NET_CMD_IFUP=y +CONFIG_CMD_MIITOOL=y CONFIG_CMD_PING=y CONFIG_CMD_TFTP=y CONFIG_CMD_EDIT=y CONFIG_CMD_SPLASH=y +CONFIG_CMD_FBTEST=y CONFIG_CMD_READLINE=y CONFIG_CMD_TIMEOUT=y CONFIG_CMD_CLK=y +CONFIG_CMD_DETECT=y CONFIG_CMD_FLASH=y CONFIG_CMD_GPIO=y CONFIG_CMD_LED=y CONFIG_CMD_LED_TRIGGER=y +CONFIG_CMD_OF_NODE=y +CONFIG_CMD_OF_PROPERTY=y +CONFIG_CMD_OF_DISPLAY_TIMINGS=y +CONFIG_CMD_OF_FIXUP_STATUS=y CONFIG_CMD_OFTREE=y CONFIG_NET=y CONFIG_NET_NFS=y CONFIG_OF_BAREBOX_DRIVERS=y CONFIG_DRIVER_NET_MACB=y +CONFIG_DAVICOM_PHY=y # CONFIG_SPI is not set CONFIG_MTD=y # CONFIG_MTD_OOB_DEVICE is not set -- 2.12.0 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH v4 1/3] phylib: add Davicom PHY support
Based on driver from Linux kernel 4.18.0-rc4
Signed-off-by: Sam Ravnborg
---
drivers/net/phy/Kconfig | 5 ++
drivers/net/phy/Makefile | 1 +
drivers/net/phy/davicom.c | 140 ++
3 files changed, 146 insertions(+)
create mode 100644 drivers/net/phy/davicom.c
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index cda752b65..79fb917ee 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -18,6 +18,11 @@ config AT803X_PHY
---help---
Currently supports the AT8030, AT8031 and AT8035 PHYs.
+config DAVICOM_PHY
+ bool "Driver for Davicom PHYs"
+ ---help---
+ Currently supports dm9161e and dm9131
+
config LXT_PHY
bool "Driver for the Intel LXT PHYs"
---help---
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 30b20f8ee..4424054d9 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -1,6 +1,7 @@
obj-y += phy.o mdio_bus.o
obj-$(CONFIG_AR8327N_PHY) += ar8327.o
obj-$(CONFIG_AT803X_PHY) += at803x.o
+obj-$(CONFIG_DAVICOM_PHY) += davicom.o
obj-$(CONFIG_LXT_PHY) += lxt.o
obj-$(CONFIG_MARVELL_PHY) += marvell.o
obj-$(CONFIG_MICREL_PHY) += micrel.o
diff --git a/drivers/net/phy/davicom.c b/drivers/net/phy/davicom.c
new file mode 100644
index 0..8a784b1e5
--- /dev/null
+++ b/drivers/net/phy/davicom.c
@@ -0,0 +1,140 @@
+/*
+ * drivers/net/phy/davicom.c
+ *
+ * Driver for Davicom PHYs
+ *
+ * Author: Andy Fleming
+ *
+ * Copyright (c) 2004 Freescale Semiconductor, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#define MII_DM9161_SCR 0x10
+#define MII_DM9161_SCR_INIT0x0610
+#define MII_DM9161_SCR_RMII0x0100
+
+/* DM9161 Interrupt Register */
+#define MII_DM9161_INTR0x15
+#define MII_DM9161_INTR_PEND 0x8000
+#define MII_DM9161_INTR_DPLX_MASK 0x0800
+#define MII_DM9161_INTR_SPD_MASK 0x0400
+#define MII_DM9161_INTR_LINK_MASK 0x0200
+#define MII_DM9161_INTR_MASK 0x0100
+#define MII_DM9161_INTR_DPLX_CHANGE0x0010
+#define MII_DM9161_INTR_SPD_CHANGE 0x0008
+#define MII_DM9161_INTR_LINK_CHANGE0x0004
+#define MII_DM9161_INTR_INIT 0x
+#define MII_DM9161_INTR_STOP \
+(MII_DM9161_INTR_DPLX_MASK | MII_DM9161_INTR_SPD_MASK \
+ | MII_DM9161_INTR_LINK_MASK | MII_DM9161_INTR_MASK)
+
+/* DM9161 10BT Configuration/Status */
+#define MII_DM9161_10BTCSR 0x12
+#define MII_DM9161_10BTCSR_INIT0x7800
+
+MODULE_DESCRIPTION("Davicom PHY driver");
+MODULE_AUTHOR("Andy Fleming");
+MODULE_LICENSE("GPL");
+
+
+static int dm9161_config_aneg(struct phy_device *phydev)
+{
+ int err;
+
+ /* Isolate the PHY */
+ err = phy_write(phydev, MII_BMCR, BMCR_ISOLATE);
+
+ if (err < 0)
+ return err;
+
+ /* Configure the new settings */
+ err = genphy_config_aneg(phydev);
+
+ if (err < 0)
+ return err;
+
+ return 0;
+}
+
+static int dm9161_config_init(struct phy_device *phydev)
+{
+ int err, temp;
+
+ /* Isolate the PHY */
+ err = phy_write(phydev, MII_BMCR, BMCR_ISOLATE);
+
+ if (err < 0)
+ return err;
+
+ switch (phydev->interface) {
+ case PHY_INTERFACE_MODE_MII:
+ temp = MII_DM9161_SCR_INIT;
+ break;
+ case PHY_INTERFACE_MODE_RMII:
+ temp = MII_DM9161_SCR_INIT | MII_DM9161_SCR_RMII;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ /* Do not bypass the scrambler/descrambler */
+ err = phy_write(phydev, MII_DM9161_SCR, temp);
+ if (err < 0)
+ return err;
+
+ /* Clear 10BTCSR to default */
+ err = phy_write(phydev, MII_DM9161_10BTCSR, MII_DM9161_10BTCSR_INIT);
+
+ if (err < 0)
+ return err;
+
+ /* Reconnect the PHY, and enable Autonegotiation */
+ return phy_write(phydev, MII_BMCR, BMCR_ANENABLE);
+}
+
+static struct phy_driver dm91xx_driver[] = {
+{
+ .phy_id = 0x0181b880,
+ .drv.name = "Davicom DM9161E",
+ .phy_id_mask= 0x0ff0,
+ .features = PHY_BASIC_FEATURES,
+ .config_init= dm9161_config_init,
+ .config_aneg= dm9161_config_aneg,
+}, {
+ .phy_id = 0x0181b8b0,
+ .drv.name = "Davicom DM9161B/C",
+ .phy_id_mask= 0x0ff0,
+ .features = PHY_BASIC_FEATURES,
+ .config_init= dm9161_config_init,
+ .config_aneg= dm9161_config_aneg,
+}, {
+ .phy_id = 0x0181b8a0,
+ .drv.name = "Davicom DM9161A",
+ .phy_id_mask= 0
[PATCH v4 2/3] phylib: add support for reset-gpios
Add minimal support for reset-gpios in the PHY node.
Example DT that uses this:
macb0: ethernet@fffbc000 {
phy-mode = "rmii";
#address-cells = <1>;
#size-cells = <0>;
ethphy0: ethernet-phy@1 {
reg = <3>;
reset-gpios = <&pioE 17 GPIO_ACTIVE_LOW>;
reset-assert-us = <1000>;
reset-deassert-us = <2000>;
};
};
The reset is required to get the Davicom PHY operational on
my proprietary board, and is assumed relevant for other boards too.
The PHY is reset when we read the info from DT,
before the phy_id is retreived.
The bindings are documented in dts/Bindings/net/phy.txt.
A side-effect of this patch is that we may see
phy devices created from the DT due to the extra call to:
of_mdiobus_register()
with the ethernet node as argument.
The logic to determine if a child node is a PHY node
is a simpler version compared to the one used in the kernel.
The kernel have a whitelist of compatible strings
that is not included in the barebox version.
They can be added later if needed.
Signed-off-by: Sam Ravnborg
---
drivers/net/phy/mdio_bus.c | 96 +-
1 file changed, 95 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index d209716a1..636bdd1db 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -17,14 +17,19 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include
+#include
#include
#include
+#include
#include
#include
#include
#include
#include
+#define DEFAULT_GPIO_RESET_ASSERT 1000 /* us */
+#define DEFAULT_GPIO_RESET_DEASSERT 1000 /* us */
+
LIST_HEAD(mii_bus_list);
int mdiobus_detect(struct device_d *dev)
@@ -82,6 +87,82 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio,
struct device_node *chi
return 0;
}
+/*
+ * Node is considered a PHY node if:
+ * o Compatible string of "ethernet-phy-idX.X"
+ * o Compatible string of "ethernet-phy-ieee802.3-c45"
+ * o Compatible string of "ethernet-phy-ieee802.3-c22"
+ * o No compatibility string
+ *
+ * A device which is not a phy is expected to have a compatible string
+ * indicating what sort of device it is.
+ */
+static bool of_mdiobus_child_is_phy(struct device_node *np)
+{
+ struct property *prop;
+ const char *cp;
+
+ of_property_for_each_string(np, "compatible", prop, cp) {
+if (!strncmp(cp, "ethernet-phy-id", strlen("ethernet-phy-id")))
+ return true;
+}
+
+ if (of_device_is_compatible(np, "ethernet-phy-ieee802.3-c45"))
+ return true;
+
+ if (of_device_is_compatible(np, "ethernet-phy-ieee802.3-c22"))
+ return true;
+
+ if (!of_find_property(np, "compatible", NULL))
+ return true;
+
+ return false;
+}
+
+/*
+ * Reset the PHY, based on DT info
+ *
+ * If np is a phy node, and the phy node contains a reset-gpios property
+ * then reset the phy.
+ */
+static void of_mdiobus_reset_phy(struct mii_bus *bus, struct device_node *np)
+{
+ enum of_gpio_flags of_flags;
+ u32 reset_deassert_delay;
+ u32 reset_assert_delay;
+ unsigned long flags;
+ int gpio;
+ int ret;
+
+ gpio = of_get_named_gpio_flags(np, "reset-gpios", 0, &of_flags);
+ if (!gpio_is_valid(gpio))
+ return;
+
+ flags = GPIOF_OUT_INIT_INACTIVE;
+ if (of_flags & OF_GPIO_ACTIVE_LOW)
+ flags |= GPIOF_ACTIVE_LOW;
+
+ ret = gpio_request_one(gpio, flags, np->name);
+ if (ret) {
+ dev_err(&bus->dev, "failed to request reset gpio for: %s\n",
+ np->name);
+ return;
+ }
+
+ reset_assert_delay = DEFAULT_GPIO_RESET_ASSERT;
+ of_property_read_u32(np, "reset-assert-us", &reset_assert_delay);
+
+ reset_deassert_delay = DEFAULT_GPIO_RESET_DEASSERT;
+ of_property_read_u32(np, "reset-deassert-us", &reset_deassert_delay);
+
+ /* reset the PHY */
+ dev_dbg(&bus->dev, "reset PHY with GPIO: %d\n", gpio);
+ gpio_set_active(gpio, 1);
+ udelay(reset_assert_delay);
+ gpio_set_active(gpio, 0);
+ udelay(reset_deassert_delay);
+}
+
/**
* of_mdiobus_register - Register mii_bus and create PHYs from the device tree
* @mdio: pointer to mii_bus structure
@@ -111,6 +192,10 @@ static int of_mdiobus_register(struct mii_bus *mdio,
struct device_node *np)
continue;
}
+ if (!of_mdiobus_child_is_phy(child))
+ continue;
+
+ of_mdiobus_reset_phy(mdio, child);
of_mdiobus_register_phy(mdio, child, addr);
}
@@ -154,8 +239,17 @@ int mdiobus_register(struct mii_bus *bus)
pr_info("%s: probed\n", dev_name(&bus->dev
[PATCH v4 0/3] Add Davicom phy + reset-gpios
Changes in v4:
- Either register a mdio node or phy child nodes
- Check for valid phy node before registering the phy node
This is like the kernel does it
- Also recognize compatible="ethernet-phy-id*" as PHY's
again like the kernel, but with a simpler implementation,
as we do not try to read the actual phy_id
Changes in v3:
- Consider the v3 a new implmentation - almost nothing was
left from v2. Thus also invalidate any former review - sorry.
- Dropped the need for the mdio {} node, as this node is
only used when there is dedicated HW support for the mdio.
- Added so PHY child nodes to ethernet nodes are registered,
and as part of registration the PHY nodes are reset if
the reset-gpios property is present.
- Further dropped that the parsed info is stored in the bus,
as we only do reset once, thus there is no need to
save the info from the DT
- Futher dropped helper function to reset. They was not needed
- Named the gpio with the name of the PHY node
in the DT. This makes it unique and easier to recognize.
Changes in v2:
- Added patch to enable Davicom PHY on at91sam9263ek - evaluation kit
- Fix so we do reset before comunicating with the PHY
- Rename to mdio_reset()
- Reference correct binding file in commit log (mdio.txt)
- Tested on at91sam9263ek
The at91sam9263ek kit do not require the reset like my
proprietary board, so no DT changes required
Intro:
The following patches was necessary to get networking
operational on my proprietary target.
The target is at91sam9263 based with a Davicom PHY.
The Davicom PHY is a straight copy form the Linux
kernel with the interrupt routine removed and
minor adjustments to the rest.
The davicom PHY would not work until it had seen a reset
cycle - which I think may be an artifact of the board design.
To fix the reset issue I have implemented support for the
reset-gpios binding (see net/phy.txt bindings).
A minimal implmentation was done, just enough to get
my target running.
I could have implemented something in macb -
but I preferred the more generic solution.
Also included are a patch that for the at91sam9263ek
evaluation board. The patch adds several extra tools
that are usefull for testing, and enable the Davicom PHY.
Sam
Sam Ravnborg (3):
phylib: add Davicom PHY support
phylib: add support for reset-gpios
at91sam9263ek: add PHY, miitool etc. to config
arch/arm/configs/at91sam9263ek_defconfig | 12 ++-
drivers/net/phy/Kconfig | 5 ++
drivers/net/phy/Makefile | 1 +
drivers/net/phy/davicom.c| 140 +++
drivers/net/phy/mdio_bus.c | 97 -
5 files changed, 253 insertions(+), 2 deletions(-)
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH v3 2/3] phylib: add support for reset-gpios
Hi Ahmad. On Wed, Aug 22, 2018 at 03:14:57PM +0200, Ahmad Fatoum wrote: > Hello Sam, > > On 08/21/2018 09:46 PM, Sam Ravnborg wrote: > > But macb.txt or the general bindigns do not mention > > a mdio child. > > And the Linux macb_main.c do not support a "mdio" node either. > > So my best guess is that the "mdio" support in macb was accidently > > ported over from fec-imx when adding DT support. > [snip] > > Considering the above I will do the following: > > a) Remove "mdio" support from macb.c > > While submitting a macb patch to linux-netdev, I was asked [1] to add > support for a "mdio" subtree in the macb binding. I intend to submit > it when net-next opens for inclusion into Linux v4.20. > That way, macb would look at phys in the list of its children as well > as the children of its "mdio" child. > > Just a heads up to avoid removing functionality that would've to be > reinstated later on... > > [1]: https://www.spinics.net/lists/netdev/msg519196.html Thanks for bringing this to my attention. I will leave the code as-is. Sam ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH v3 2/3] phylib: add support for reset-gpios
Hello Sam, On 08/21/2018 09:46 PM, Sam Ravnborg wrote: > But macb.txt or the general bindigns do not mention > a mdio child. > And the Linux macb_main.c do not support a "mdio" node either. > So my best guess is that the "mdio" support in macb was accidently > ported over from fec-imx when adding DT support. [snip] > Considering the above I will do the following: > a) Remove "mdio" support from macb.c While submitting a macb patch to linux-netdev, I was asked [1] to add support for a "mdio" subtree in the macb binding. I intend to submit it when net-next opens for inclusion into Linux v4.20. That way, macb would look at phys in the list of its children as well as the children of its "mdio" child. Just a heads up to avoid removing functionality that would've to be reinstated later on... [1]: https://www.spinics.net/lists/netdev/msg519196.html Cheers Ahmad ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 3/4] ratp: implement support for GPIO commands
On Tue, Aug 21, 2018 at 05:20:00PM +0200, Aleksander Morgado wrote:
> Introduce three new RATP commands that allow getting and setting GPIO
> values as well as configuring the direction of the GPIO pins.
>
Same here as with the i2c bus/address thing. Being to able to use names for
the GPIOs would be a good thing.
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +struct ratp_bb_gpio_get_value_request {
> + struct ratp_bb header;
> + uint32_t gpio;
> +} __attribute__((packed));
Nitpick: I prefer not to align the variable names in struct definitions.
If the next element with a longer type is added then you can only give
up the alignment or patch unrelated lines (which is bad because 'git
blame' gives no useful output for the changed lines)
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 1/4] ratp: implement i2c read/write support
On Tue, Aug 21, 2018 at 05:19:58PM +0200, Aleksander Morgado wrote:
> Introduce two new RATP commands that allow running i2c read/write
> operations, very similar in format to the already existing md/mw
> RATP commands.
>
> The messages are defined with a fixed 16-bit long register field, but
> it will only be treated as a 16-bit address if I2C_FLAG_WIDE_ADDRESS
> is set in the message flags field. If this flag is unset, the start
> register address is assumed 8-bit long.
>
> If the message includes the I2C_FLAG_MASTER_MODE flag, the start
> register field is ignored and a i2c master send/receive operation is
> performed.
>
> Signed-off-by: Aleksander Morgado
> ---
> common/ratp/Makefile | 1 +
> common/ratp/i2c.c| 281 +++
> include/ratp_bb.h| 4 +
> 3 files changed, 286 insertions(+)
> create mode 100644 common/ratp/i2c.c
>
> diff --git a/common/ratp/Makefile b/common/ratp/Makefile
> index 2c6d674f6..0234b55c1 100644
> --- a/common/ratp/Makefile
> +++ b/common/ratp/Makefile
> @@ -4,3 +4,4 @@ obj-y += getenv.o
> obj-y += md.o
> obj-y += mw.o
> obj-y += reset.o
> +obj-y += i2c.o
> diff --git a/common/ratp/i2c.c b/common/ratp/i2c.c
> new file mode 100644
> index 0..b8d055b67
> --- /dev/null
> +++ b/common/ratp/i2c.c
> @@ -0,0 +1,281 @@
> +/*
> + * Copyright (c) 2011-2018 Sascha Hauer , Pengutronix
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +/* NOTE:
> + * - Fixed-size fields (e.g. integers) are given just after the header.
> + * - Variable-length fields are stored inside the buffer[] and their
> position
> + *within the buffer[] and their size are given as fixed-sized fields
> after
> + *the header.
> + * The message may be extended at any time keeping backwards compatibility,
> + * as the position of the buffer[] is given by the buffer_offset field. i.e.
> + * increasing the buffer_offset field we can extend the fixed-sized section
> + * to add more fields.
> + */
> +
> +#define I2C_FLAG_WIDE_ADDRESS (1 << 0)
> +#define I2C_FLAG_MASTER_MODE (1 << 1)
> +
> +struct ratp_bb_i2c_read_request {
> + struct ratp_bb header;
> + uint16_t buffer_offset;
> + uint8_t bus;
> + uint8_t addr;
I wonder how we see the RATP support. If it's for adhoc debugging then
bus/addr is fine. The caller should have no expectations that the bus
number is constant though. Likewise for the address which might change
across different board revisions.
Should we have support for resolving names, which could be provided by
aliases in dt?
We could still add name resolving support later as a separate call, I
just thought that now is the time to think how we proceed.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
Re: i2c master send/receive mode
On Tue, Aug 21, 2018 at 05:18:24PM +0200, Aleksander Morgado wrote: > The i2c master send support was already implemented in the i2c_write command, > but it was not properly documented. The first patch in the series addresses > that. > > In the second patch, the i2c master receive mode is implemented in the > i2c_read command. > > [PATCH 1/2] i2c_write: document master send mode > [PATCH 2/2] i2c_read: implement support for master receive mode Applied, thanks Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH] bbremote: add missing 'md' packet handler
On Tue, Aug 21, 2018 at 05:07:00PM +0200, Aleksander Morgado wrote:
> Signed-off-by: Aleksander Morgado
> ---
> scripts/remote/controller.py | 3 +++
> 1 file changed, 3 insertions(+)
Applied, thanks
Sascha
>
> diff --git a/scripts/remote/controller.py b/scripts/remote/controller.py
> index 2ed834613..1a8390904 100644
> --- a/scripts/remote/controller.py
> +++ b/scripts/remote/controller.py
> @@ -46,6 +46,9 @@ def unpack(data):
> elif p_type == BBType.fs_return:
> logging.debug("received: fs_return")
> return BBPacketFSReturn(raw=data)
> +elif p_type == BBType.md:
> +logging.debug("received: md")
> +return BBPacketMd(raw=data)
> elif p_type == BBType.md_return:
> logging.debug("received: md_return")
> return BBPacketMdReturn(raw=data)
> --
> 2.18.0
>
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH] Revert "i.MX: Add provisions to boot from IRAM"
On Mon, Aug 20, 2018 at 11:41:07PM -0700, Andrey Smirnov wrote:
> After being introduced 3 years ago this feature ended up being
> "obsoleted by events" and project it was supposed to be a part of
> winded down.
>
> Revert this feature due to:
>
> a) Lack of users
>
> b) Existence of better way to make barebox load via SRAM as
> intermediary step that does not require two separate images to be
> built (.imx-sram-img)
>
> This reverts commit 903c9477a08c5655161779ef4144886928ecc7d1.
>
> Signed-off-by: Andrey Smirnov
> ---
Applied, thanks
Sascha
> Documentation/boards/imx.rst | 27 -
> .../arm/boards/freescale-mx51-babbage/board.c | 60 ---
> .../flash-header-common.imxcfg| 58 --
> .../flash-header-imx51-babbage-xload.imxcfg | 3 -
> .../flash-header-imx51-babbage.imxcfg | 60 ++-
> .../boards/freescale-mx51-babbage/lowlevel.c | 25
> arch/arm/configs/imx_v7-xload_defconfig | 31 --
> arch/arm/mach-imx/Kconfig | 15 -
> arch/arm/mach-imx/Makefile| 1 -
> arch/arm/mach-imx/xload.c | 52
> images/Makefile.imx | 27 ++---
> 11 files changed, 63 insertions(+), 296 deletions(-)
> delete mode 100644
> arch/arm/boards/freescale-mx51-babbage/flash-header-common.imxcfg
> delete mode 100644
> arch/arm/boards/freescale-mx51-babbage/flash-header-imx51-babbage-xload.imxcfg
> delete mode 100644 arch/arm/configs/imx_v7-xload_defconfig
> delete mode 100644 arch/arm/mach-imx/xload.c
>
> diff --git a/Documentation/boards/imx.rst b/Documentation/boards/imx.rst
> index 56fd3ab41..99ca10b7c 100644
> --- a/Documentation/boards/imx.rst
> +++ b/Documentation/boards/imx.rst
> @@ -118,33 +118,6 @@ Some notes about the mentioned *conditions*.
> - ``until_any_bit_clear`` waits until ``(*addr & mask) != mask`` is true
> - ``until_any_bit_set`` waits until ``(*addr & mask) != 0`` is true.
>
> -Internal Boot Mode Through Internal RAM(IRAM)
> --
> -
> -The Internal Boot Mode Through Internal RAM is supported on:
> -
> -* i.MX51
> -
> -As can be easily deduced from its name, the Internal Boot Mode Through
> -Internal RAM is just a variant of Internal Boot Mode so all of the
> -stated above still applies in this case. What it differs in is the following:
> -
> -* Boot process is done in two stages(First stage binary can be
> - produced with ``imx_v7-xload_defconfig``)
> -* DCD of the first stage image is set such that the image is fetched
> - into an unoccupied area or IRAM
> -* First stage image once uncompressed and set up will look for a
> - second stage bootloader on the same media it booted from and start
> - it(see mach-imx/xload.c for more details)
> -* Second stage images are just regular i.MX boot images
> -
> -Since on a typical i.MX SoC unused IRAM area is not enough to run
> -anything but a PBL this mode, due to its very limited usability,
> -serves only one purpose -- allow for a portion of a bootloader to be
> -executed without depending on DRAM to be functional. This peculiarity
> -of the mode can be used to implement various memory testing
> -scenarious.
> -
> USB Boot
>
>
> diff --git a/arch/arm/boards/freescale-mx51-babbage/board.c
> b/arch/arm/boards/freescale-mx51-babbage/board.c
> index 996c3d2e4..74f93f654 100644
> --- a/arch/arm/boards/freescale-mx51-babbage/board.c
> +++ b/arch/arm/boards/freescale-mx51-babbage/board.c
> @@ -135,63 +135,3 @@ static int imx51_babbage_init(void)
> return 0;
> }
> coredevice_initcall(imx51_babbage_init);
> -
> -#ifdef CONFIG_ARCH_IMX_XLOAD
> -
> -static int imx51_babbage_xload_init_pinmux(void)
> -{
> - static const iomux_v3_cfg_t pinmux[] = {
> - /* (e)CSPI */
> - MX51_PAD_CSPI1_MOSI__ECSPI1_MOSI,
> - MX51_PAD_CSPI1_MISO__ECSPI1_MISO,
> - MX51_PAD_CSPI1_SCLK__ECSPI1_SCLK,
> -
> - /* (e)CSPI chip select lines */
> - MX51_PAD_CSPI1_SS1__GPIO4_25,
> -
> -
> - /* eSDHC 1 */
> - MX51_PAD_SD1_CMD__SD1_CMD,
> - MX51_PAD_SD1_CLK__SD1_CLK,
> - MX51_PAD_SD1_DATA0__SD1_DATA0,
> - MX51_PAD_SD1_DATA1__SD1_DATA1,
> - MX51_PAD_SD1_DATA2__SD1_DATA2,
> - MX51_PAD_SD1_DATA3__SD1_DATA3,
> - };
> -
> - mxc_iomux_v3_setup_multiple_pads(ARRAY_AND_SIZE(pinmux));
> -
> - return 0;
> -}
> -coredevice_initcall(imx51_babbage_xload_init_pinmux);
> -
> -static int imx51_babbage_xload_init_devices(void)
> -{
> - static int spi0_chipselects[] = {
> - IMX_GPIO_NR(4, 25),
> - };
> -
> - static struct spi_imx_master spi0_pdata = {
> - .chipselect = spi0_chipselects,
> - .num_chipselect = ARRAY_SIZE(spi0_chipselects),
> - };
> -
> - static const struct spi_board_inf
Re: [PATCH 1/2] clk: clk-sccg-pll: Remove leftover debug output
On Mon, Aug 20, 2018 at 11:28:52PM -0700, Andrey Smirnov wrote:
> Signed-off-by: Andrey Smirnov
> ---
> drivers/clk/imx/clk-sccg-pll.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/clk/imx/clk-sccg-pll.c b/drivers/clk/imx/clk-sccg-pll.c
> index 951234367..bbfd95a11 100644
> --- a/drivers/clk/imx/clk-sccg-pll.c
> +++ b/drivers/clk/imx/clk-sccg-pll.c
> @@ -121,11 +121,9 @@ static void clk_pll1_unprepare(struct clk *clk)
> {
> struct clk_sccg_pll *pll = to_clk_sccg_pll(clk);
> u32 val;
> -printf("%s %p\n", __func__, pll);
> val = readl(pll->base);
> val |= (1 << PLL_PD);
> writel(val, pll->base);
> -printf("fuschi\n");
> }
Applied this one, thanks
I'm glad I haven't used more explicit words as debug markers ;)
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 2/2] clk: clk-sccg-pll: Drop prepare/unprepare for SCCG_PLL2
On Mon, Aug 20, 2018 at 11:28:53PM -0700, Andrey Smirnov wrote:
> A number of PLL pairs (e.g. "sys1_pll1" and "sys1_pll2") share the
> same configuration register, so touching PD bit, as is done for
> SCCG_PLL2 in its prepare/unprepare methods will result in shut down of
> both PLLs. This is very undesireable, since attempting to re-parent a
> clock to "sys1_pll2" might result in complete system shutdown due to
> "sys1_pll1" being shut-down as a part of re-parenting process.
I can imagine that there are problems with the way it is currently
handled, but the scenario you describe shouldn't happen. "sys1_pll1"
will never be shut down because it doesn't have a disable hook:
static const struct clk_ops clk_sccg_pll1_ops = {
.is_enabled = clk_pll1_is_prepared,
.recalc_rate= clk_pll1_recalc_rate,
.round_rate = clk_pll1_round_rate,
.set_rate = clk_pll1_set_rate,
};
static const struct clk_ops clk_sccg_pll2_ops = {
.enable = clk_pll1_prepare,
.disable= clk_pll1_unprepare,
.recalc_rate= clk_pll2_recalc_rate,
.round_rate = clk_pll2_round_rate,
.set_rate = clk_pll2_set_rate,
};
Have I missed something?
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH] net: Do not route traffic to interfaces that are not up
On Mon, Aug 20, 2018 at 11:28:19PM -0700, Andrey Smirnov wrote:
> In the case when:
>
> - Board has multiple network interfaces
>
> - Two ore more of those interfaces are statically configured to be
> on the same network
>
> - Only one of those interfaces is up and it is preceeded (as far as
> for_each_netdev is concerned) by interface in the same network
> that isn't
>
> net_route() will choose "non-up" device as a route for traffic
> resulting in no network connectivity. Change the routing logic to also
> consider if interface is "up", so that only such interfaces would be
> considered for sending network traffic out.
>
> Signed-off-by: Andrey Smirnov
> ---
Applied, thanks
Sascha
> net/net.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/net.c b/net/net.c
> index d21855415..63f42fa5c 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -149,7 +149,7 @@ struct eth_device *net_route(IPaddr_t dest)
> struct eth_device *edev;
>
> for_each_netdev(edev) {
> - if (!edev->ipaddr)
> + if (!edev->ipaddr || !edev->ifup)
> continue;
>
> if ((dest & edev->netmask) == (edev->ipaddr & edev->netmask)) {
> --
> 2.17.1
>
>
> ___
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH] include/common: Make use of ALIGN and ALIGN_DOWN
On Mon, Aug 20, 2018 at 11:27:36PM -0700, Andrey Smirnov wrote: > Signed-off-by: Andrey Smirnov > --- > include/common.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Applied, thanks Sascha > > diff --git a/include/common.h b/include/common.h > index f93bd7f5d..abbe73fd3 100644 > --- a/include/common.h > +++ b/include/common.h > @@ -109,8 +109,8 @@ void shutdown_barebox(void); > > #define PAGE_SIZE4096 > #define PAGE_SHIFT 12 > -#define PAGE_ALIGN(s) (((s) + PAGE_SIZE - 1) & ~(PAGE_SIZE - 1)) > -#define PAGE_ALIGN_DOWN(x) ((x) & ~(PAGE_SIZE - 1)) > +#define PAGE_ALIGN(s)ALIGN(s, PAGE_SIZE) > +#define PAGE_ALIGN_DOWN(x) ALIGN_DOWN(x, PAGE_SIZE) > > int memory_display(const void *addr, loff_t offs, unsigned nbytes, int size, > int swab); > > -- > 2.17.1 > > > ___ > barebox mailing list > barebox@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/barebox > -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH] ARM: cache-l2x0: Make use of IS_ALIGNED and ALIGN_DOWN
On Mon, Aug 20, 2018 at 11:27:17PM -0700, Andrey Smirnov wrote:
> Signed-off-by: Andrey Smirnov
> ---
> arch/arm/cpu/cache-l2x0.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
Applied, thanks
Sascha
>
> diff --git a/arch/arm/cpu/cache-l2x0.c b/arch/arm/cpu/cache-l2x0.c
> index 8e0fff66d..e975ecffc 100644
> --- a/arch/arm/cpu/cache-l2x0.c
> +++ b/arch/arm/cpu/cache-l2x0.c
> @@ -60,14 +60,14 @@ static inline void l2x0_inv_all(void)
>
> static void l2x0_inv_range(unsigned long start, unsigned long end)
> {
> - if (start & (CACHE_LINE_SIZE - 1)) {
> - start &= ~(CACHE_LINE_SIZE - 1);
> + if (!IS_ALIGNED(start, CACHE_LINE_SIZE)) {
> + start = ALIGN_DOWN(start, CACHE_LINE_SIZE);
> l2x0_flush_line(start);
> start += CACHE_LINE_SIZE;
> }
>
> - if (end & (CACHE_LINE_SIZE - 1)) {
> - end &= ~(CACHE_LINE_SIZE - 1);
> + if (!IS_ALIGNED(end, CACHE_LINE_SIZE)) {
> + end = ALIGN_DOWN(end, CACHE_LINE_SIZE);
> l2x0_flush_line(end);
> }
>
> @@ -87,7 +87,7 @@ static void l2x0_clean_range(unsigned long start, unsigned
> long end)
> {
> void __iomem *base = l2x0_base;
>
> - start &= ~(CACHE_LINE_SIZE - 1);
> + start = ALIGN_DOWN(start, CACHE_LINE_SIZE);
> while (start < end) {
> unsigned long blk_end = start + min(end - start, 4096UL);
>
> @@ -102,7 +102,7 @@ static void l2x0_clean_range(unsigned long start,
> unsigned long end)
>
> static void l2x0_flush_range(unsigned long start, unsigned long end)
> {
> - start &= ~(CACHE_LINE_SIZE - 1);
> + start = ALIGN_DOWN(start, CACHE_LINE_SIZE);
> while (start < end) {
> unsigned long blk_end = start + min(end - start, 4096UL);
>
> --
> 2.17.1
>
>
> ___
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 19/22] bbu: Remove logical negation in barebox_update_handler_exists()
On Mon, Aug 20, 2018 at 11:26:00PM -0700, Andrey Smirnov wrote: > Returning !bbu_find_handler() from barebox_update_handler_exists() > would return the opposite result from what the name of that funciton > implies. Drop the "!" to make it behave as expected. > > Signed-off-by: Andrey Smirnov > --- > common/bbu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/common/bbu.c b/common/bbu.c > index 11e44f4a7..69ccac68a 100644 > --- a/common/bbu.c > +++ b/common/bbu.c > @@ -151,7 +151,7 @@ bool barebox_update_handler_exists(struct bbu_data *data) > if (!data->handler_name) > return false; > > - return !bbu_find_handler(data->handler_name); > + return bbu_find_handler(data->handler_name); As bbu_find_handler() returns a pointer maybe better '!!' or bbu_find_handler() != NULL? Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 18/22] ARM: i.MX: bbu: Adjust error code check for pwrite()
On Mon, Aug 20, 2018 at 11:25:59PM -0700, Andrey Smirnov wrote:
> Pwrite() will return the amount bytes written or negative error code
> on success, so we need to do two things with it:
>
> 1. Check it against "image_len" to make sure we actually wrote all
>of the data
>
> 2. Set it to zero in case of success, since that is what code in
>barebox_update() expects to happen
>
> Signed-off-by: Andrey Smirnov
> ---
> arch/arm/mach-imx/imx-bbu-internal.c | 11 +--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-imx/imx-bbu-internal.c
> b/arch/arm/mach-imx/imx-bbu-internal.c
> index d83eb972c..70af5ef84 100644
> --- a/arch/arm/mach-imx/imx-bbu-internal.c
> +++ b/arch/arm/mach-imx/imx-bbu-internal.c
> @@ -86,6 +86,7 @@ static int imx_bbu_write_device(struct
> imx_internal_bbu_handler *imx_handler,
> const void *buf, int image_len)
> {
> int fd, ret, offset = 0;
> + bool partial_write;
>
> fd = open(devicefile, O_RDWR | O_CREAT);
> if (fd < 0)
> @@ -117,8 +118,14 @@ static int imx_bbu_write_device(struct
> imx_internal_bbu_handler *imx_handler,
> }
>
> ret = pwrite(fd, buf, image_len, offset);
> - if (ret < 0)
> + partial_write = ret > 0 && ret != image_len;
> + if (ret < 0 || partial_write) {
> + ret = partial_write ? -EIO : ret;
> +
> + pr_err("writing to %s failed with %s\n", devicefile,
> +strerror(-ret));
> goto err_close;
Do we need a pwrite_full analog to write_full?
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
