Skip to contentSkip to conversation

OMAP4: clock: shrink clock data utilizing preprocessor.

RFC PATCH New Tony Lindgren 5 participants 11 messages May 13, 2011 1 message not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

#1 lore raw

RFC PATCH 1/1 OMAP4: clock: shrink clock data utilizing preprocessor.

Quoted 8 lines · Vladimir Zapolskiy

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.

---
 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

#2 lore raw

RFC PATCH 1/1 OMAP4: clock: shrink clock data utilizing preprocessor.

Quoted 10 lines · Tony Lindgren
Quoted 8 lines · Vladimir Zapolskiy

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.

---
?arch/arm/mach-omap2/clock44xx_data.c | 3313 +++++++++------------------------- ?1 files changed, 848 insertions(+), 2465 deletions(-)

Quoted 6 lines

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.

Quoted 4 lines

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

#3 lore raw

RFC PATCH 1/1 OMAP4: clock: shrink clock data utilizing preprocessor.

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.

Quoted 13 lines · Vladimir Zapolskiy
Quoted 12 lines · Tony Lindgren
Quoted 8 lines · Vladimir Zapolskiy

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.

---
  arch/arm/mach-omap2/clock44xx_data.c | 3313 +++++++++-------------------------
  1 files changed, 848 insertions(+), 2465 deletions(-)

Hehehe, the diffstat certainly looks good :)

Quoted 7 lines
Quoted 4 lines

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

#4 lore raw

RFC PATCH 1/1 OMAP4: clock: shrink clock data utilizing preprocessor.

Quoted 10 lines · Cousson, Benoit
Quoted 5 lines
Quoted 2 lines
Quoted 1 line

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

#5 lore raw

RFC PATCH 1/1 OMAP4: clock: shrink clock data utilizing preprocessor.

Quoted 12 lines · Tony Lindgren
Quoted 10 lines · Cousson, Benoit
Quoted 5 lines
Quoted 2 lines
Quoted 1 line

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.

Quoted 7 lines

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

#6 lore raw

RFC PATCH 1/1 OMAP4: clock: shrink clock data utilizing preprocessor.

Quoted 22 lines · Kevin Hilman
Quoted 12 lines · Tony Lindgren
Quoted 10 lines · Cousson, Benoit
Quoted 5 lines
Quoted 2 lines
Quoted 1 line

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.

Quoted 7 lines

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..

Quoted 8 lines

... 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

#7 lore raw

RFC PATCH 1/1 OMAP4: clock: shrink clock data utilizing preprocessor.

In reply to Kevin Hilman
Quoted 22 lines · Kevin
Quoted 12 lines · Tony Lindgren
Quoted 10 lines · Cousson, Benoit
Quoted 5 lines
Quoted 2 lines
Quoted 1 line

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.

Quoted 7 lines

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..

Quoted 8 lines

... 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

#8 lore raw

RFC PATCH 1/1 OMAP4: clock: shrink clock data utilizing preprocessor.

Quoted 31 lines · Benoit
Quoted 30 lines · Kevin
Quoted 14 lines · Tony Lindgren<[email protected]> ?writes:
Quoted 8 lines · Cousson, Benoit
Quoted 7 lines
Quoted 3 lines
Quoted 2 lines

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.

Quoted 4 lines

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.

Quoted 7 lines

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.

Quoted 6 lines
Quoted 3 lines

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.

Quoted 4 lines

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.

Quoted 4 lines

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 :)

Quoted 4 lines

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

With best wishes,
Vladimir

#9 lore raw

RFC PATCH 1/1 OMAP4: clock: shrink clock data utilizing preprocessor.

In reply to Kevin Hilman
Quoted 36 lines

-----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.

Quoted 4 lines · Tony Lindgren
Quoted 3 lines · Cousson, Benoit
Quoted 3 lines
Quoted 2 lines
Quoted 1 line

Unfortunately I don't have an automated tool, but that

would be great

Quoted 1 line
Quoted 1 line
Quoted 1 line

to have such a script. For this time I've checked the

correctness of the

Quoted 7 lines
Quoted 6 lines
Quoted 1 line

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.

Quoted 7 lines

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.

Quoted 7 lines

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?

Quoted 10 lines

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

#10 lore raw

RFC PATCH 1/1 OMAP4: clock: shrink clock data utilizing preprocessor.

Quoted 38 lines · Sanjeev
Quoted 38 lines

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.

Quoted 4 lines · Tony Lindgren
Quoted 3 lines · Cousson, Benoit
Quoted 3 lines
Quoted 2 lines
Quoted 1 line

Unfortunately I don't have an automated tool, but that

would be great

Quoted 1 line
Quoted 1 line
Quoted 1 line

to have such a script. For this time I've checked the

correctness of the

Quoted 7 lines
Quoted 6 lines
Quoted 1 line

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.

Quoted 7 lines

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.

Quoted 6 lines
Quoted 4 lines

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

#11 lore raw

RFC PATCH 1/1 OMAP4: clock: shrink clock data utilizing preprocessor.

In reply to Tony Lindgren
Quoted 12 lines · Tony Lindgren
Quoted 8 lines · Vladimir Zapolskiy

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.

---
 arch/arm/mach-omap2/clock44xx_data.c | 3313 +++++++++-------------------------
 1 files changed, 848 insertions(+), 2465 deletions(-)

Hehehe, the diffstat certainly looks good :)

Quoted 7 lines

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