[PATCH] regulator: Provide a check for dummy regulator

[PATCH] regulator: Provide a check for dummy regulator

[Jassi Brar]

Usually changing the regulator output involves delays before/after the
operation.
There are consumer drivers shared by platforms, where some may
not really have a regulator in the path. Which causes the consumer
to unnecessarily (sometimes disruptively) incur delays for the
"dummy" regulator.
Since the 'struct regulator' is opaque outside of the core,
provide a function to check if the given regulator is a dummy one.

Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
 drivers/regulator/core.c           |    9 +++++++++
 include/linux/regulator/consumer.h |    6 ++++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index e70dd38..2fb6e5b 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3115,6 +3115,15 @@ void regulator_use_dummy_regulator(void)
 EXPORT_SYMBOL_GPL(regulator_use_dummy_regulator);
 
 /**
+ * regulator_is_dummy - Check if the regulator is the placeholder 'dummy'
+ */
+bool regulator_is_dummy(struct regulator *regulator)
+{
+	return !strcmp(regulator->rdev->desc->name, "dummy");
+}
+EXPORT_SYMBOL_GPL(regulator_is_dummy);
+
+/**
  * rdev_get_drvdata - get rdev regulator driver data
  * @rdev: regulator
  *
diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index 4ed1b30..7b0360c 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -141,6 +141,7 @@ void regulator_put(struct regulator *regulator);
 void devm_regulator_put(struct regulator *regulator);
 
 /* regulator output control and status */
+bool regulator_is_dummy(struct regulator *regulator);
 int regulator_enable(struct regulator *regulator);
 int regulator_disable(struct regulator *regulator);
 int regulator_force_disable(struct regulator *regulator);
@@ -221,6 +222,11 @@ static inline void devm_regulator_put(struct regulator *regulator)
 {
 }
 
+static bool regulator_is_dummy(struct regulator *regulator)
+{
+	return true;
+}
+
 static inline int regulator_enable(struct regulator *regulator)
 {
 	return 0;
-- 
1.7.4.1

Re: [PATCH] regulator: Provide a check for dummy regulator

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 2781 bytes --]

On Thu, Apr 19, 2012 at 03:21:37PM +0530, Jassi Brar wrote:
> Usually changing the regulator output involves delays before/after the
> operation.
> There are consumer drivers shared by platforms, where some may
> not really have a regulator in the path. Which causes the consumer
> to unnecessarily (sometimes disruptively) incur delays for the
> "dummy" regulator.

This analysis doesn't sound quite right - if it's the dummy regulator
then obviously these delays don't happen so presumably the cost is
actually coming from the rdev mutex and recursion up the regulator tree
- the basic overheads of calling into the regulator API.

> Since the 'struct regulator' is opaque outside of the core,
> provide a function to check if the given regulator is a dummy one.

No, this isn't great from a usability and abstraction point of view.  

In usability terms this sort of performance optimisation is going to be
desired by a wide range of drivers (and wouldn't hurt those that don't
urgently need it) so we shouldn't force every user to open code the use
of this information.  Worse, the whole point of the dummy regulator is
that it allows users to not worry about this sort of stuff so it'd mean
that the dummy regulator was failing to perform its only function.

In abstraction terms it's not the fact that it's a dummy regulator
that's interesting here but rather the fact that the regulator doesn't
have control the consumer can use and there's a whole raft of other
reasons why that might be the case.  The constraints may not permit
status changes, or a real regulator may not physically support enable
and disable operations (eg, if there's a GPIO for enable and it's tied
on all the time).  If there's a useful performance win for dummy
regulators it'll apply equally well to all these other cases so we
should't be special casing dummy regulators.

I've just sent out an untested patch (you're CCed) which should give a
substantial win for the enable/disable case which will hopefully address
the issue for you.  If you're concerned about voltage change rather than
enable/disable I'd like to understand better exactly where the
performance is going but we can certainly do a similar fast path for
fixed voltage regulators.  I'd be surprised if consumers that need to
change voltages played nicely with the fixed voltage regulator while
using it often enough for anyone to care about performance.

I really don't understand why people are so keen to special case things
like this in individual consumers, not sure what we can do to encourage
more generic fixes.  There was a guy from Qualcomm the other week who
was absolutely insistent that we had to do some fragile special case
stuff with supplies for similar reasons :(

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] regulator: Provide a check for dummy regulator

[Jassi Brar]

On 19 April 2012 18:12, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> On Thu, Apr 19, 2012 at 03:21:37PM +0530, Jassi Brar wrote:
>> Usually changing the regulator output involves delays before/after the
>> operation.
>> There are consumer drivers shared by platforms, where some may
>> not really have a regulator in the path. Which causes the consumer
>> to unnecessarily (sometimes disruptively) incur delays for the
>> "dummy" regulator.
>
> This analysis doesn't sound quite right - if it's the dummy regulator
> then obviously these delays don't happen so presumably the cost is
> actually coming from the rdev mutex and recursion up the regulator tree
> - the basic overheads of calling into the regulator API.
>
Sorry, I didn't explain well.

I am not talking about the time the regulator output needs to stabilize or the
time taken by control to traverse the regulator tree for "dummy".

I am talking about the delay the consumer driver might want after
enabling the regulator for the end device to, say, initialize itself or
perform POST or whatever.
Now the consumer driver is shared by two platforms - one that has
direct supply to the device that turns on with main power supply
and another platform that has indeed a regulator that is turned
off by default and the consumer driver enables it during probe
and then has to wait tens of millisecs for end device's POST (say).

I do realize that such consumer drivers ought to know about the
presence of the real regulator via platform_data(PD) or DT, but
if we already don't expect consumers to know from PD/DT that
a regulator doesn't exist and hence never even need a "dummy",
maybe the regulator core could lessen the number of PD/DT
nodes by also telling if a given regulator is a real or a dummy one.

That way dummy could actually play a 'positive' role - right now
it only excuses platforms that are lazy to neither define a regulator
nor tell consumer via PD/DT that there is none.

Another POV is :
  Is a consumer's need, to know if the gotten regulator is a real
or a dummy one, reasonable ?

>
> I've just sent out an untested patch (you're CCed) which should give a
> substantial win for the enable/disable case which will hopefully address
> the issue for you.
>
The patch does address an issue, but not mine :)

Regards,
Jassi

Re: [PATCH] regulator: Provide a check for dummy regulator

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1353 bytes --]

On Thu, Apr 19, 2012 at 07:35:03PM +0530, Jassi Brar wrote:

> I am talking about the delay the consumer driver might want after
> enabling the regulator for the end device to, say, initialize itself or
> perform POST or whatever.

> I do realize that such consumer drivers ought to know about the
> presence of the real regulator via platform_data(PD) or DT, but

No!  You're *completely* failing to understand the usage model of the
regulator API here, any driver doing this is clearly buggy.

> Another POV is :
>   Is a consumer's need, to know if the gotten regulator is a real
> or a dummy one, reasonable ?

Absolutely not, you're completely failing to understand what I said
about abstraction here.  *Nothing* about this is anything to do with
dummy regulators in the slightest, it's about supplies which are already
on at the time the driver powers the device up.  Doing this for dummy
regulators is not helpful, being enabled isn't in the slightest bit tied
to the fact that the regulator is a dummy regulator and it's silly to
optimise this only in the specific case where a dummy regulator (which
is in the first place only a crutch to keep systems with buggy regulator
setups going) is being used is silly.

I'd suggest checking with regulator_is_enabled() prior to power up, or
adding a notifier for physical enable events and using that.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] regulator: Provide a check for dummy regulator

[Jassi Brar]

On 19 April 2012 21:59, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> On Thu, Apr 19, 2012 at 07:35:03PM +0530, Jassi Brar wrote:
>
>> Another POV is :
>>   Is a consumer's need, to know if the gotten regulator is a real
>> or a dummy one, reasonable ?
>
> Absolutely not, you're completely failing to understand what I said
> about abstraction here.  *Nothing* about this is anything to do with
> dummy regulators in the slightest, it's about supplies which are already
> on at the time the driver powers the device up.  Doing this for dummy
> regulators is not helpful, being enabled isn't in the slightest bit tied
> to the fact that the regulator is a dummy regulator and it's silly to
> optimise this only in the specific case where a dummy regulator (which
> is in the first place only a crutch to keep systems with buggy regulator
> setups going) is being used is silly.
>
> I'd suggest checking with regulator_is_enabled() prior to power up, or
> adding a notifier for physical enable events and using that.
>
Sorry for being slow.
How would you suggest one works out the following arrangement ?

Say, an audio CODEC chip has a simple PLL1 run by main voltage
domain(Vmain). PLL1 can only support 11025x sample rates.
Another PLL2 on the chip, powered by optional supply Vaux, could
provide 8000x sample rates. Obviously low-end platforms could choose
to not provide Vaux.

The platform has some other drivers that need to have dummy regulator
support enabled.

How is the CODEC driver supposed to know if 'vaux' is actually present?
Because if it isn't present, the driver wouldn't declare support for
8000x  rates.
regulator_get() would always succeed, and regulator_is_enabled() would
always return 1, even if the vaux is not supplied.

Thanks,
-Jassi

Re: [PATCH] regulator: Provide a check for dummy regulator

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1048 bytes --]

On Fri, Apr 20, 2012 at 01:02:06PM +0530, Jassi Brar wrote:

> Say, an audio CODEC chip has a simple PLL1 run by main voltage
> domain(Vmain). PLL1 can only support 11025x sample rates.
> Another PLL2 on the chip, powered by optional supply Vaux, could
> provide 8000x sample rates. Obviously low-end platforms could choose
> to not provide Vaux.

> The platform has some other drivers that need to have dummy regulator
> support enabled.

> How is the CODEC driver supposed to know if 'vaux' is actually present?
> Because if it isn't present, the driver wouldn't declare support for
> 8000x  rates.
> regulator_get() would always succeed, and regulator_is_enabled() would
> always return 1, even if the vaux is not supplied.

This is just one of those cases that won't ever work well with dummy
regulators - the whole reason you're supposed to actually specify the
supplies accurately and not use them is that we've got no way of telling
if the supply there or not from consumer drivers as we just
unconditionally claim that the supply is there.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] regulator: Provide a check for dummy regulator

[Jassi Brar]

On 20 April 2012 17:16, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Apr 20, 2012 at 01:02:06PM +0530, Jassi Brar wrote:
>
>> Say, an audio CODEC chip has a simple PLL1 run by main voltage
>> domain(Vmain). PLL1 can only support 11025x sample rates.
>> Another PLL2 on the chip, powered by optional supply Vaux, could
>> provide 8000x sample rates. Obviously low-end platforms could choose
>> to not provide Vaux.
>
>> The platform has some other drivers that need to have dummy regulator
>> support enabled.
>
>> How is the CODEC driver supposed to know if 'vaux' is actually present?
>> Because if it isn't present, the driver wouldn't declare support for
>> 8000x  rates.
>> regulator_get() would always succeed, and regulator_is_enabled() would
>> always return 1, even if the vaux is not supplied.
>
> This is just one of those cases that won't ever work well with dummy
> regulators
>
That's what I have been trying to fix. My example might be fictitious but
I have a real scenario with omap_hsmmc (I was avoiding getting into that).

The dummy not just provide a place holder data structure for missing definition
of an available supply, but it also masks the fact that there might indeed be no
supply at all on the given machine.

As I said, atm the only option for a consumer is to know it via PD/DT.

We can avoid requiring this 'oob' info by simply letting the consumer driver
know that  "The platform didn't provide the supply, but here is a dummy
one in case you don't strictly require your supplies be completely defined
by all platforms".
Which can be achieved by simply adding that regulator_is_dummy() call.

The benefit is magnified by the fact that, for a given circuit, at
least theoretically,
there is no limit to the number/combination of supplies that could be controlled
by inserting a regulator. And that could lead to a very noisy PD/DT.

Thanks,
-Jassi

Re: [PATCH] regulator: Provide a check for dummy regulator

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1456 bytes --]

On Fri, Apr 20, 2012 at 05:59:42PM +0530, Jassi Brar wrote:

> That's what I have been trying to fix. My example might be fictitious but
> I have a real scenario with omap_hsmmc (I was avoiding getting into that).

> The dummy not just provide a place holder data structure for missing definition
> of an available supply, but it also masks the fact that there might indeed be no
> supply at all on the given machine.

> As I said, atm the only option for a consumer is to know it via PD/DT.

No.  You're failing to understand what dummy regulators are for.  To
repeat yet again you're not supposed to actually use dummy regulators,
you're supposed to fully specify the regulators on your platform.  As
I've said several times now dummy regulators are just a crutch to hold
systems together, if they're not working out the solution is to turn
them off and even if they are working out turning them off is still the
best thing to do.  The problems you are seeing are exactly why this is
the case.

> The benefit is magnified by the fact that, for a given circuit, at
> least theoretically,
> there is no limit to the number/combination of supplies that could be controlled
> by inserting a regulator. And that could lead to a very noisy PD/DT.

No, any given circuit is going to have a very clear set of things that
can be controlled by a regulator, and most chips have a fixed set of
supplies they always need so it's very simple from their point of view.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] regulator: Provide a check for dummy regulator

[Jassi Brar]

On 20 April 2012 18:31, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Apr 20, 2012 at 05:59:42PM +0530, Jassi Brar wrote:
>
>> That's what I have been trying to fix. My example might be fictitious but
>> I have a real scenario with omap_hsmmc (I was avoiding getting into that).
>
>> The dummy not just provide a place holder data structure for missing definition
>> of an available supply, but it also masks the fact that there might indeed be no
>> supply at all on the given machine.
>
>> As I said, atm the only option for a consumer is to know it via PD/DT.
>
> No.  You're failing to understand what dummy regulators are for.
>
I think I do understand what dummy regulators are for and also that
they are meant to go away some time in future.

>  To repeat yet again you're not supposed to actually use dummy regulators,
> you're supposed to fully specify the regulators on your platform.
>
I use dummy regulators for what they are meant for - until every platform
completely define supplies for every consumer.
We are not there yet, yet one 'ideal' consumer suffers because of that.

If you ask me to go away and disable dummy and update all consumers
and their platforms, then I say why not remove the concept of dummy
altogether which only delays full compliance to the regulator api ?

-j

Re: [PATCH] regulator: Provide a check for dummy regulator

[Jassi Brar]

On 20 April 2012 19:18, Jassi Brar <jaswinder.singh@linaro.org> wrote:
>
>>  To repeat yet again you're not supposed to actually use dummy regulators,
>> you're supposed to fully specify the regulators on your platform.
>>
> I use dummy regulators for what they are meant for - until every platform
> completely define supplies for every consumer.
> We are not there yet, yet one 'ideal' consumer suffers because of that.
>
> If you ask me to go away and disable dummy and update all consumers
> and their platforms, then I say why not remove the concept of dummy
> altogether which only delays full compliance to the regulator api ?
>
Btw, by 'all platforms' I meant those based on my SoC. Popular term - machines.

Re: [PATCH] regulator: Provide a check for dummy regulator

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 2486 bytes --]

On Fri, Apr 20, 2012 at 07:18:15PM +0530, Jassi Brar wrote:
> On 20 April 2012 18:31, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> > No.  You're failing to understand what dummy regulators are for.

> I think I do understand what dummy regulators are for and also that
> they are meant to go away some time in future.

No, they're here to stay - people will always be bringing up new boards
after all - but they're not supposed to be used on platforms.

> >  To repeat yet again you're not supposed to actually use dummy regulators,
> > you're supposed to fully specify the regulators on your platform.

> I use dummy regulators for what they are meant for - until every platform
> completely define supplies for every consumer.
> We are not there yet, yet one 'ideal' consumer suffers because of that.

> If you ask me to go away and disable dummy and update all consumers
> and their platforms, then I say why not remove the concept of dummy
> altogether which only delays full compliance to the regulator api ?

The only reason dummy regulators are there is to provide a get out of
jail card to help keep systems booting when there's a problem with their
regulator setup.  Managing the introduction of regulator support to new
drivers is relatively hard, I don't see us getting it right first time
every time, so it's unlikely we'll ever be able to get rid of these
crutches.  It keeps people happy so even though the regulator setup is
generally trivial to fix when an issue crops up it's less hassle to have
the option.

In any case, as I've said it's not the fact that you've got a dummy
supply that's an issue for most of the cases you mention - it's the fact
that the supply is always on.  

Even in the case where a supply might genuinely be missing (which are
very unusual, it's more effort in silicon to cope with missing supplies)
you're still stuck with guessing if the supply is missing because of
missing regulator setup or if the supply really is not physically
present.  If you really want to try to bodge in something for thingns
like this in the consumer probably a good solution is to do something
like have the driver check that it can get a sensible voltage back
though I'd be interested to see a driver that can usefully use it and a
system which did so.  Your example seemed pretty theoretical and it'd be
more usual to tie to a supply so I'd really like to look at some
concrete systems that do this.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] regulator: Provide a check for dummy regulator

[Jassi Brar]

On 20 April 2012 20:12, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
>
>> > No.  You're failing to understand what dummy regulators are for.
>
>> I think I do understand what dummy regulators are for and also that
>> they are meant to go away some time in future.
>
> No, they're here to stay - people will always be bringing up new boards
> after all - but they're not supposed to be used on platforms.
>
"some time in future" means utopia.... whenever that be. The longer that is
to more important is it for dummy support to be 'complete'.

>
> In any case, as I've said it's not the fact that you've got a dummy
> supply that's an issue for most of the cases you mention - it's the fact
> that the supply is always on.
>
I am not sure anymore you read my posts :(
I am more bothered by the case where the supply is always off i.e,
physically absent.

> Even in the case where a supply might genuinely be missing (which are
> very unusual, it's more effort in silicon to cope with missing supplies)
>
It's not that unusual.  See vcc_aux at line 150 of omap_hsmmc.c

See how dummy fools the driver throughout into stuff like toggling vcc
even for platforms that don't really provide vcc_aux.
In practice, see how dummy affects (negatively because of unnecessary
toggles) the vcc too.  Not every platform with omap_hsmmc might afford
having dummy disabled (I don't know all to tell for sure).

Best Regards,
-Jassi

Re: [PATCH] regulator: Provide a check for dummy regulator

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 3105 bytes --]

On Fri, Apr 20, 2012 at 11:55:22PM +0530, Jassi Brar wrote:
> On 20 April 2012 20:12, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> > No, they're here to stay - people will always be bringing up new boards
> > after all - but they're not supposed to be used on platforms.

> "some time in future" means utopia.... whenever that be. The longer that is
> to more important is it for dummy support to be 'complete'.

Well, for any given board it's very quick to resolve.  It's just that
from time to time people will break things and then they'll get annoyed
(and probably spend forever complaining on the lists rather than the
brief time taken to make the patch).

> > In any case, as I've said it's not the fact that you've got a dummy
> > supply that's an issue for most of the cases you mention - it's the fact
> > that the supply is always on.

> I am not sure anymore you read my posts :(
> I am more bothered by the case where the supply is always off i.e,
> physically absent.

It's not what you first mentioned...  In any case, the solution is very
clear - just do proper regulator support and turn off dummy regulators
and everything is fine.

> > Even in the case where a supply might genuinely be missing (which are
> > very unusual, it's more effort in silicon to cope with missing supplies)

> It's not that unusual.  See vcc_aux at line 150 of omap_hsmmc.c

No, really - that's exceptionally unusual and quite honestly given the
"discussion" surrounding the addition of the regulator support there I'm
not convinced that anything the driver is doing here is sane; the whole
stuff with regulator_get_exclusive() never made any sense for MMC in the
first place (and has now been removed).

> See how dummy fools the driver throughout into stuff like toggling vcc
> even for platforms that don't really provide vcc_aux.
> In practice, see how dummy affects (negatively because of unnecessary
> toggles) the vcc too.  Not every platform with omap_hsmmc might afford
> having dummy disabled (I don't know all to tell for sure).

The fix here is in the boards, not with the dummy code.  Open coding
handling of this sort of stuff in individual consumers is just insane,
and you're always going to run into the situation where you don't know
if the dummy regulator is there because the board setup is incomplete or
because someone left the option turned on when it shouldn't be.

What you want here is something like regulator_get_real_no_really()
which never returns a dummy regulator but the whole thing is just adding
fragility onto bodge.  I'm not overly concerned if systems don't run as
efficiently as they might when using dummy regulators because (like I
say) they're just a crutch to keep systems running.

Note also that the performance improvement stuff I sent the other day
will help here, the bounces will be very cheap now.

I'd much rather remove the feature than start going down this rabbit
hole, it's just going to be a world of pain and fragility.  But then a
whole different bunch of people will complain.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] regulator: Provide a check for dummy regulator

[Jassi Brar]

On 21 April 2012 00:18, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

Going for the closure.
I have a last nail. Don't know whose coffin is it for ;)

>
> In any case, the solution is very clear - just do proper regulator
> support and turn off dummy regulators and everything is fine.
>
You earlier said there are always going to be 'new' platforms that would
need dummy enabled.
Now just imagine if one of those platforms happen to share the consumer
due to which you suggest disabling the dummy (omap_hsmmc in this case).
If we re-enable dummy, then your's isn't really a solution.
If we require that platform to come with full compliance, why can't we
ask every platform to do the same?

Thanks,
-Jassi

Re: [PATCH] regulator: Provide a check for dummy regulator

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1395 bytes --]

On Sat, Apr 21, 2012 at 12:41:18AM +0530, Jassi Brar wrote:
> On 21 April 2012 00:18, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> > In any case, the solution is very clear - just do proper regulator
> > support and turn off dummy regulators and everything is fine.

> You earlier said there are always going to be 'new' platforms that would
> need dummy enabled.

When I say "need" I mean either "are at the point where just booting is
massive success so who cares if there's the odd bit of breakage" or
"have been randomly broken and need investigation as to what went
wrong".

> Now just imagine if one of those platforms happen to share the consumer
> due to which you suggest disabling the dummy (omap_hsmmc in this case).
> If we re-enable dummy, then your's isn't really a solution.
> If we require that platform to come with full compliance, why can't we
> ask every platform to do the same?

We do ask every platform to do the same, but enough people run into
problems that it's been useful to provide a standard crutch people can
pick up to get their systems up and running.  The theory is that dummy
regulators are enabled to help keep things running and as a debugging
aid, they're not intended for ongoing use.  If nothing else the log
message that gets printed out whenever a dummy regulator is used can be
helpful in identifying what's missing from the board setup.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]