[PATCH] [RFC] OMAP4: clock: shrink clock data utilizing preprocessor.

[PATCH] [RFC] OMAP4: clock: shrink clock data utilizing preprocessor.

[Tony Lindgren]

* Vladimir Zapolskiy <vzapolskiy@gmail.com> [110512 17:35]:
> This mass change reduces homogeneous data chunks along clock
> definitions. No semantical difference is added to the change,
> and still it could be introduced easily.
> 
> Signed-off-by: Vladimir Zapolskiy <vzapolskiy@gmail.com>
> ---
>  arch/arm/mach-omap2/clock44xx_data.c | 3313 +++++++++-------------------------
>  1 files changed, 848 insertions(+), 2465 deletions(-)

Hehehe, the diffstat certainly looks good :)

This would be best done for all of them using some
perl/sed/python script. And could be done for the hwmod data
too. Do you already have something like that available?

Paul and & Benoit, what's your take on doing something like
this? I'd assume updating your data generation scripts would
be trivial?

Regards,

Tony

[PATCH] [RFC] OMAP4: clock: shrink clock data utilizing preprocessor.

[Vladimir Zapolskiy]

On Fri, May 13, 2011 at 2:30 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Vladimir Zapolskiy <vzapolskiy@gmail.com> [110512 17:35]:
>> This mass change reduces homogeneous data chunks along clock
>> definitions. No semantical difference is added to the change,
>> and still it could be introduced easily.
>>
>> Signed-off-by: Vladimir Zapolskiy <vzapolskiy@gmail.com>
>> ---
>> ?arch/arm/mach-omap2/clock44xx_data.c | 3313 +++++++++-------------------------
>> ?1 files changed, 848 insertions(+), 2465 deletions(-)
>
> Hehehe, the diffstat certainly looks good :)
>
> This would be best done for all of them using some
> perl/sed/python script. And could be done for the hwmod data
> too. Do you already have something like that available?
>
Unfortunately I don't have an automated tool, but that would be great
to have such a script. For this time I've checked the correctness of the
change comparing the preprocessed output.

In fact if nobody has serious objections about the nature of the change
itself, then I'd like to continue with similar modifications for other OMAP
specific massive data files, hopefully there is a lot of redundant data to
eliminate :)

I can try to do that starting from writing the script for automation,
though I don't have a strong feeling that it's a straightforward task,
and preprocessed code check and/or runtime check might be preferable.

> Paul and & Benoit, what's your take on doing something like
> this? I'd assume updating your data generation scripts would
> be trivial?
>

With best wishes,
Vladimir

[PATCH] [RFC] OMAP4: clock: shrink clock data utilizing preprocessor.

[Kevin Hilman]

Tony Lindgren <tony@atomide.com> writes:

> * Vladimir Zapolskiy <vzapolskiy@gmail.com> [110512 17:35]:
>> This mass change reduces homogeneous data chunks along clock
>> definitions. No semantical difference is added to the change,
>> and still it could be introduced easily.
>> 
>> Signed-off-by: Vladimir Zapolskiy <vzapolskiy@gmail.com>
>> ---
>>  arch/arm/mach-omap2/clock44xx_data.c | 3313 +++++++++-------------------------
>>  1 files changed, 848 insertions(+), 2465 deletions(-)
>
> Hehehe, the diffstat certainly looks good :)
>
> This would be best done for all of them using some
> perl/sed/python script. And could be done for the hwmod data
> too. Do you already have something like that available?
>
> Paul and & Benoit, what's your take on doing something like
> this? I'd assume updating your data generation scripts would
> be trivial?

Can this be reposted to the list?  I didn't receive the original, and
don't see it in the l-o or l-a-k archives.

Thanks,

Kevin

[PATCH] [RFC] OMAP4: clock: shrink clock data utilizing preprocessor.

[Cousson, Benoit]

Hi Vladimir,

Could you please re-send your original patch?

For some reason I cannot find it in any mailing list so far.
Maybe it is due to its size.

On 5/13/2011 2:08 PM, Vladimir Zapolskiy wrote:
> On Fri, May 13, 2011 at 2:30 PM, Tony Lindgren<tony@atomide.com>  wrote:
>> * Vladimir Zapolskiy<vzapolskiy@gmail.com>  [110512 17:35]:
>>> This mass change reduces homogeneous data chunks along clock
>>> definitions. No semantical difference is added to the change,
>>> and still it could be introduced easily.
>>>
>>> Signed-off-by: Vladimir Zapolskiy<vzapolskiy@gmail.com>
>>> ---
>>>   arch/arm/mach-omap2/clock44xx_data.c | 3313 +++++++++-------------------------
>>>   1 files changed, 848 insertions(+), 2465 deletions(-)
>>
>> Hehehe, the diffstat certainly looks good :)
>>
>> This would be best done for all of them using some
>> perl/sed/python script. And could be done for the hwmod data
>> too. Do you already have something like that available?
>>
> Unfortunately I don't have an automated tool, but that would be great
> to have such a script. For this time I've checked the correctness of the
> change comparing the preprocessed output.

In fact these files are already generated automatically, as written in 
the header file. So changing the output format should straightforward. 
At least for OMAP4... OMAP2 and OMAP3 were done manually some time ago.

Regards,
Benoit

[PATCH] [RFC] OMAP4: clock: shrink clock data utilizing preprocessor.

[Tony Lindgren]

* Cousson, Benoit <b-cousson@ti.com> [110513 15:18]:
> >>>Signed-off-by: Vladimir Zapolskiy<vzapolskiy@gmail.com>
> >>
> >Unfortunately I don't have an automated tool, but that would be great
> >to have such a script. For this time I've checked the correctness of the
> >change comparing the preprocessed output.
> 
> In fact these files are already generated automatically, as written
> in the header file. So changing the output format should
> straightforward. At least for OMAP4... OMAP2 and OMAP3 were done
> manually some time ago.

Sounds like the important thing to consider here is how these macros
should be set up considering the upcoming generic clock framework
and device tree changes.

So let's wait a few days for comments from Benoit and Paul on the
format for the macros so we don't need to redo them again later.
Of course there might be other things to consider too..

Regards,

Tony

[PATCH] [RFC] OMAP4: clock: shrink clock data utilizing preprocessor.

[Kevin Hilman]

Tony Lindgren <tony@atomide.com> writes:

> * Cousson, Benoit <b-cousson@ti.com> [110513 15:18]:
>> >>>Signed-off-by: Vladimir Zapolskiy<vzapolskiy@gmail.com>
>> >>
>> >Unfortunately I don't have an automated tool, but that would be great
>> >to have such a script. For this time I've checked the correctness of the
>> >change comparing the preprocessed output.
>> 
>> In fact these files are already generated automatically, as written
>> in the header file. So changing the output format should
>> straightforward. At least for OMAP4... OMAP2 and OMAP3 were done
>> manually some time ago.
>
> Sounds like the important thing to consider here is how these macros
> should be set up considering the upcoming generic clock framework
> and device tree changes.
>
> So let's wait a few days for comments from Benoit and Paul on the
> format for the macros so we don't need to redo them again later.
> Of course there might be other things to consider too..

... like readability.

After seeing the patch (thanks Benoit), I think this is bad tradeoff
between readability and lines-of-code.

Personally, I don't think we should be trading readability for diffstat
goodness.  I have a strong dislike for these multi-line macros, but
it's up to Paul/Benoit to decide how this should look.

Kevin

[PATCH] [RFC] OMAP4: clock: shrink clock data utilizing preprocessor.

[Tony Lindgren]

* Kevin Hilman <khilman@ti.com> [110513 16:12]:
> Tony Lindgren <tony@atomide.com> writes:
> 
> > * Cousson, Benoit <b-cousson@ti.com> [110513 15:18]:
> >> >>>Signed-off-by: Vladimir Zapolskiy<vzapolskiy@gmail.com>
> >> >>
> >> >Unfortunately I don't have an automated tool, but that would be great
> >> >to have such a script. For this time I've checked the correctness of the
> >> >change comparing the preprocessed output.
> >> 
> >> In fact these files are already generated automatically, as written
> >> in the header file. So changing the output format should
> >> straightforward. At least for OMAP4... OMAP2 and OMAP3 were done
> >> manually some time ago.
> >
> > Sounds like the important thing to consider here is how these macros
> > should be set up considering the upcoming generic clock framework
> > and device tree changes.
> >
> > So let's wait a few days for comments from Benoit and Paul on the
> > format for the macros so we don't need to redo them again later.
> > Of course there might be other things to consider too..
> 
> ... like readability.
> 
> After seeing the patch (thanks Benoit), I think this is bad tradeoff
> between readability and lines-of-code.
> 
> Personally, I don't think we should be trading readability for diffstat
> goodness.  I have a strong dislike for these multi-line macros, but
> it's up to Paul/Benoit to decide how this should look.

Despite having few hard-to-read multi-line macros, this can be
used to make the actual data entries more readable. The same way as
REGULATOR_SUPPLY, OMAP3_MUX, etc.

Tony

[PATCH] [RFC] OMAP4: clock: shrink clock data utilizing preprocessor.

[Cousson, Benoit]

On 5/13/2011 3:16 PM, Hilman, Kevin wrote:
> Tony Lindgren<tony@atomide.com>  writes:
>
>> * Cousson, Benoit<b-cousson@ti.com>  [110513 15:18]:
>>>>>> Signed-off-by: Vladimir Zapolskiy<vzapolskiy@gmail.com>
>>>>>
>>>> Unfortunately I don't have an automated tool, but that would be great
>>>> to have such a script. For this time I've checked the correctness of the
>>>> change comparing the preprocessed output.
>>>
>>> In fact these files are already generated automatically, as written
>>> in the header file. So changing the output format should
>>> straightforward. At least for OMAP4... OMAP2 and OMAP3 were done
>>> manually some time ago.
>>
>> Sounds like the important thing to consider here is how these macros
>> should be set up considering the upcoming generic clock framework
>> and device tree changes.
>>
>> So let's wait a few days for comments from Benoit and Paul on the
>> format for the macros so we don't need to redo them again later.
>> Of course there might be other things to consider too..
>
> ... like readability.
>
> After seeing the patch (thanks Benoit), I think this is bad tradeoff
> between readability and lines-of-code.
>
> Personally, I don't think we should be trading readability for diffstat
> goodness.  I have a strong dislike for these multi-line macros, but
> it's up to Paul/Benoit to decide how this should look.

I'm sharing the same concern and after seeing the patch, I do thing as 
well that the readability is badly impacted.
My other concern is that these macros are too low level and does not 
bring a real abstraction of clock nodes.
Any change to the clock structure we will have to do in the near future 
with the common clock fmwk will probably have an impact on this file.

So if we want to reduce the file size using that kind of approach, we'd 
better provide improved macros that will help hiding some implementation 
details. It will ease the transition to the common clock fmwk.

Last but not least, since we do want to move this file to DT ASAP, is it 
worth it?

Regards,
Benoit

[PATCH] [RFC] OMAP4: clock: shrink clock data utilizing preprocessor.

[Premi, Sanjeev]

> -----Original Message-----
> From: linux-omap-owner at vger.kernel.org 
> [mailto:linux-omap-owner at vger.kernel.org] On Behalf Of Hilman, Kevin
> Sent: Friday, May 13, 2011 6:46 PM
> To: Tony Lindgren
> Cc: Cousson, Benoit; Vladimir Zapolskiy; Paul Walmsley; 
> linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org
> Subject: Re: [PATCH] [RFC] OMAP4: clock: shrink clock data 
> utilizing preprocessor.
> 
> Tony Lindgren <tony@atomide.com> writes:
> 
> > * Cousson, Benoit <b-cousson@ti.com> [110513 15:18]:
> >> >>>Signed-off-by: Vladimir Zapolskiy<vzapolskiy@gmail.com>
> >> >>
> >> >Unfortunately I don't have an automated tool, but that 
> would be great
> >> >to have such a script. For this time I've checked the 
> correctness of the
> >> >change comparing the preprocessed output.
> >> 
> >> In fact these files are already generated automatically, as written
> >> in the header file. So changing the output format should
> >> straightforward. At least for OMAP4... OMAP2 and OMAP3 were done
> >> manually some time ago.
> >
> > Sounds like the important thing to consider here is how these macros
> > should be set up considering the upcoming generic clock framework
> > and device tree changes.
> >
> > So let's wait a few days for comments from Benoit and Paul on the
> > format for the macros so we don't need to redo them again later.
> > Of course there might be other things to consider too..
> 
> ... like readability.
> 
> After seeing the patch (thanks Benoit), I think this is bad tradeoff
> between readability and lines-of-code.
> 
> Personally, I don't think we should be trading readability 
> for diffstat
> goodness.  I have a strong dislike for these multi-line macros, but
> it's up to Paul/Benoit to decide how this should look.

[sp] Was the patch posted again?

> 
> Kevin
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe 
> linux-omap" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH] [RFC] OMAP4: clock: shrink clock data utilizing preprocessor.

[Cousson, Benoit]

On 5/13/2011 4:48 PM, Premi, Sanjeev wrote:
>> From: linux-omap-owner at vger.kernel.org
>> [mailto:linux-omap-owner at vger.kernel.org] On Behalf Of Hilman, Kevin
>> Sent: Friday, May 13, 2011 6:46 PM
>> To: Tony Lindgren
>> Cc: Cousson, Benoit; Vladimir Zapolskiy; Paul Walmsley;
>> linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org
>> Subject: Re: [PATCH] [RFC] OMAP4: clock: shrink clock data
>> utilizing preprocessor.
>>
>> Tony Lindgren<tony@atomide.com>  writes:
>>
>>> * Cousson, Benoit<b-cousson@ti.com>  [110513 15:18]:
>>>>>>> Signed-off-by: Vladimir Zapolskiy<vzapolskiy@gmail.com>
>>>>>>
>>>>> Unfortunately I don't have an automated tool, but that
>> would be great
>>>>> to have such a script. For this time I've checked the
>> correctness of the
>>>>> change comparing the preprocessed output.
>>>>
>>>> In fact these files are already generated automatically, as written
>>>> in the header file. So changing the output format should
>>>> straightforward. At least for OMAP4... OMAP2 and OMAP3 were done
>>>> manually some time ago.
>>>
>>> Sounds like the important thing to consider here is how these macros
>>> should be set up considering the upcoming generic clock framework
>>> and device tree changes.
>>>
>>> So let's wait a few days for comments from Benoit and Paul on the
>>> format for the macros so we don't need to redo them again later.
>>> Of course there might be other things to consider too..
>>
>> ... like readability.
>>
>> After seeing the patch (thanks Benoit), I think this is bad tradeoff
>> between readability and lines-of-code.
>>
>> Personally, I don't think we should be trading readability
>> for diffstat
>> goodness.  I have a strong dislike for these multi-line macros, but
>> it's up to Paul/Benoit to decide how this should look.
>
> [sp] Was the patch posted again?

The patch size is > 100k, so it will be rejected by most mailing list.
I've just sent you the patch directly.

Regards,
Benoit

[PATCH] [RFC] OMAP4: clock: shrink clock data utilizing preprocessor.

[Vladimir Zapolskiy]

On Fri, May 13, 2011 at 5:24 PM, Cousson, Benoit <b-cousson@ti.com> wrote:
> On 5/13/2011 3:16 PM, Hilman, Kevin wrote:
>>
>> Tony Lindgren<tony@atomide.com> ?writes:
>>
>>> * Cousson, Benoit<b-cousson@ti.com> ?[110513 15:18]:
>>>>>>>
>>>>>>> Signed-off-by: Vladimir Zapolskiy<vzapolskiy@gmail.com>
>>>>>>
>>>>> Unfortunately I don't have an automated tool, but that would be great
>>>>> to have such a script. For this time I've checked the correctness of
>>>>> the
>>>>> change comparing the preprocessed output.
>>>>
>>>> In fact these files are already generated automatically, as written
>>>> in the header file. So changing the output format should
>>>> straightforward. At least for OMAP4... OMAP2 and OMAP3 were done
>>>> manually some time ago.
>>>
>>> Sounds like the important thing to consider here is how these macros
>>> should be set up considering the upcoming generic clock framework
>>> and device tree changes.
>>>
>>> So let's wait a few days for comments from Benoit and Paul on the
>>> format for the macros so we don't need to redo them again later.
>>> Of course there might be other things to consider too..
>>
>> ... like readability.
>>
>> After seeing the patch (thanks Benoit), I think this is bad tradeoff
>> between readability and lines-of-code.
>>
>> Personally, I don't think we should be trading readability for diffstat
>> goodness. ?I have a strong dislike for these multi-line macros, but
>> it's up to Paul/Benoit to decide how this should look.
>
> I'm sharing the same concern and after seeing the patch, I do thing as well
> that the readability is badly impacted.
There always shall be a price, from my side I've tried to lessen it as much as
possible.

> My other concern is that these macros are too low level and does not bring a
> real abstraction of clock nodes.
> Any change to the clock structure we will have to do in the near future with
> the common clock fmwk will probably have an impact on this file.
Right, but without this change the impact won't be less. In the best case later
for structural changes it might be simpler and less error-prone to modify one
macro definition instead of hundreds LOCs.

>
> So if we want to reduce the file size using that kind of approach, we'd
> better provide improved macros that will help hiding some implementation
> details. It will ease the transition to the common clock fmwk.
Certainly I'd be glad to get any recommendations about potential improvement
in the change, but for that case I've found quite efficient to exploit C
preprocessor for potential removing thousands LOCs without a semantic loss
or even change.

A word about my motivation, I'm just willing to reduce arch/arm/* overall size
to approach productive ARM/Linux development and integration again. At the
moment `du -sb arch/arm/* | sort -n | tail -n1` gives me mach-omap2 as the
best candidate to spend some free time with. And that change was such an
attempt, hopefully not completely fruitless and not the last one :)

>
> Last but not least, since we do want to move this file to DT ASAP, is it
> worth it?
>

With best wishes,
Vladimir