Re: [U-Boot] [PATCH] arm_cortexa8: support cache flush to other soc
OK. Will do > -Original Message- > From: Tom [mailto:tom@windriver.com] > Sent: Thursday, September 10, 2009 3:02 PM > To: Paulraj, Sandeep > Cc: Dirk Behme; u-boot@lists.denx.de; Minkyu Kang > Subject: Re: [U-Boot] [PATCH] arm_cortexa8: support cache flush to other > soc > > > Sandeep, > Can you push this change to uboot-ti ? > Tom > > >> 1) From the previous discussion I think we should apply > >> > >> http://lists.denx.de/pipermail/u-boot/2009-August/058492.html > >> > > > Tom wrote: > > Dirk Behme wrote: > >> Tom wrote: > >>> Minkyu Kang wrote: > >>>> Dear Dirk, > >>>> > >>>> > >>> > >>> > >>> I have lost track of this thread. > >> Yes, and I'm currently trying to get the track back ;) > >> > >>> When last I worked this, it seemed like the cache routines were going > to > >>> be split. > >>> > >>> 1. generic cache routines > >>> 2. legacy soc cache routines. > >>> > >>> Are the generic cache routines in place and can you use them? > >>> Else can you handle your soc specific cache routines as part of a > >>> legacy cache routine ? > >>> > >>> The omap cache routines are dependent on omap rom code and fitting > >>> new routines in using the omap specifics may not be the best way to > >>> go. > >> I'm sure this is not the perfect way, but it seems to me that splitting > >> all this stuff into several small steps would be the easier for all. > E.g. > >> > >> 1) From the previous discussion I think we should apply > >> > >> http://lists.denx.de/pipermail/u-boot/2009-August/058492.html > >> > >> Independent of any discussion if this code is needed at all, if it is > >> generic or not etc. the main advantage I understood is that it frees > the > >> way for Samsung. > >> > >> 2a) Then, what I understood from Minkyu, we need an additional patch > >> (discussion?) how to switch CONFIG_L2_OFF from compile time to run time > >> selection (for Samsung's multi board support) > >> > >> 2b) Then, what I understood from Minkyu, we should discuss about > removal > >> of get_device_type() function > >> > >> 3) In parallel, we should discuss how to further improve the OMAP3 > cache > >> stuff. What might be generic, what not, what isn't necessary etc. > >> > >> 4) Regarding (cache) code duplication, I vote for doing this > duplication > >> first. That is, have working Samsung and OMAP3 code applied in > parallel. > >> While Jean-Christophe might cry "code duplication", I learned from > OMAP3 > >> boards that is was easier to unify common code _after_ code duplication > >> was done and therefore can be easily identified. Discussing about > >> possible code duplication without being able to see (and test) the code > >> is sometimes a little tricky ;) > >> > >> As mentioned, doing this in several, small steps I feel more > comfortable > >> with. If we would have done step (1) already, it's my feeling that we > >> would have reached step 2 or 3 already. But now, we are still > discussing > >> about the 'one big perfect patch'. > >> > >> Best regards > >> > >> Dirk > >> > >> > > > > I am making this workflow up as I go.. but it seems like the > > way to resolve this is to work through the new sub-arm repo's > > > > #1 should go to u-boot-ti first and then I will will merge it to u-boot- > arm > > Then u-boot-samsung can sync to it and we will all be at a good > > starting point for #2. > > Can #1 go now to u-boot-ti ? > > If there is a merge problem, kick it back to the developer ;) > > > > This patch moves the cache support out of A8 and into omap3 which is > > the correct place for it. > > > > I assume the samsung changes are going to go first to u-boot-samsung > > Correct ? > > > > For 2a) runtime cache enabling / disabling > > New feature I don't think omap needs so it should be at some board or > > soc level that does not impact omap. > > > > For 2b) get_device_type. This is an omap-ism that goes away with #1 > > > > The means though that samsung needs its own cache routines. > > If they are going to do 2a) they will need them anyway. > > > > For 3) I don't think omap needs improving at this point. > > > > For 4) Lets see how much the samsung cache routines look like the > > omap once they are done. If it is a lot of cut-n-paste this is > > not worth arguing about a common routine will be easier to manage. > > If the cache code looks really different then it can live at the > > board or soc layer. As long as no one claims the cpu layer at the > > very least everyone's board will work. > > > > > > Tom > > > > > > ___ > > U-Boot mailing list > > U-Boot@lists.denx.de > > http://lists.denx.de/mailman/listinfo/u-boot > ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm_cortexa8: support cache flush to other soc
Sandeep, Can you push this change to uboot-ti ? Tom >> 1) From the previous discussion I think we should apply >> >> http://lists.denx.de/pipermail/u-boot/2009-August/058492.html >> Tom wrote: > Dirk Behme wrote: >> Tom wrote: >>> Minkyu Kang wrote: Dear Dirk, >>> >>> >>> I have lost track of this thread. >> Yes, and I'm currently trying to get the track back ;) >> >>> When last I worked this, it seemed like the cache routines were going to >>> be split. >>> >>> 1. generic cache routines >>> 2. legacy soc cache routines. >>> >>> Are the generic cache routines in place and can you use them? >>> Else can you handle your soc specific cache routines as part of a >>> legacy cache routine ? >>> >>> The omap cache routines are dependent on omap rom code and fitting >>> new routines in using the omap specifics may not be the best way to >>> go. >> I'm sure this is not the perfect way, but it seems to me that splitting >> all this stuff into several small steps would be the easier for all. E.g. >> >> 1) From the previous discussion I think we should apply >> >> http://lists.denx.de/pipermail/u-boot/2009-August/058492.html >> >> Independent of any discussion if this code is needed at all, if it is >> generic or not etc. the main advantage I understood is that it frees the >> way for Samsung. >> >> 2a) Then, what I understood from Minkyu, we need an additional patch >> (discussion?) how to switch CONFIG_L2_OFF from compile time to run time >> selection (for Samsung's multi board support) >> >> 2b) Then, what I understood from Minkyu, we should discuss about removal >> of get_device_type() function >> >> 3) In parallel, we should discuss how to further improve the OMAP3 cache >> stuff. What might be generic, what not, what isn't necessary etc. >> >> 4) Regarding (cache) code duplication, I vote for doing this duplication >> first. That is, have working Samsung and OMAP3 code applied in parallel. >> While Jean-Christophe might cry "code duplication", I learned from OMAP3 >> boards that is was easier to unify common code _after_ code duplication >> was done and therefore can be easily identified. Discussing about >> possible code duplication without being able to see (and test) the code >> is sometimes a little tricky ;) >> >> As mentioned, doing this in several, small steps I feel more comfortable >> with. If we would have done step (1) already, it's my feeling that we >> would have reached step 2 or 3 already. But now, we are still discussing >> about the 'one big perfect patch'. >> >> Best regards >> >> Dirk >> >> > > I am making this workflow up as I go.. but it seems like the > way to resolve this is to work through the new sub-arm repo's > > #1 should go to u-boot-ti first and then I will will merge it to u-boot-arm > Then u-boot-samsung can sync to it and we will all be at a good > starting point for #2. > Can #1 go now to u-boot-ti ? > If there is a merge problem, kick it back to the developer ;) > > This patch moves the cache support out of A8 and into omap3 which is > the correct place for it. > > I assume the samsung changes are going to go first to u-boot-samsung > Correct ? > > For 2a) runtime cache enabling / disabling > New feature I don't think omap needs so it should be at some board or > soc level that does not impact omap. > > For 2b) get_device_type. This is an omap-ism that goes away with #1 > > The means though that samsung needs its own cache routines. > If they are going to do 2a) they will need them anyway. > > For 3) I don't think omap needs improving at this point. > > For 4) Lets see how much the samsung cache routines look like the > omap once they are done. If it is a lot of cut-n-paste this is > not worth arguing about a common routine will be easier to manage. > If the cache code looks really different then it can live at the > board or soc layer. As long as no one claims the cpu layer at the > very least everyone's board will work. > > > Tom > > > ___ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm_cortexa8: support cache flush to other soc
Dear Tom, 2009/9/9 Tom : > Minkyu Kang wrote: >> Tom wrote: >>> Dirk Behme wrote: Tom wrote: > Minkyu Kang wrote: >> Dear Dirk, >> >> > > > I have lost track of this thread. Yes, and I'm currently trying to get the track back ;) > When last I worked this, it seemed like the cache routines were going to > be split. > > 1. generic cache routines > 2. legacy soc cache routines. > > Are the generic cache routines in place and can you use them? > Else can you handle your soc specific cache routines as part of a > legacy cache routine ? > > The omap cache routines are dependent on omap rom code and fitting > new routines in using the omap specifics may not be the best way to > go. I'm sure this is not the perfect way, but it seems to me that splitting all this stuff into several small steps would be the easier for all. E.g. 1) From the previous discussion I think we should apply http://lists.denx.de/pipermail/u-boot/2009-August/058492.html Independent of any discussion if this code is needed at all, if it is generic or not etc. the main advantage I understood is that it frees the way for Samsung. 2a) Then, what I understood from Minkyu, we need an additional patch (discussion?) how to switch CONFIG_L2_OFF from compile time to run time selection (for Samsung's multi board support) 2b) Then, what I understood from Minkyu, we should discuss about removal of get_device_type() function 3) In parallel, we should discuss how to further improve the OMAP3 cache stuff. What might be generic, what not, what isn't necessary etc. 4) Regarding (cache) code duplication, I vote for doing this duplication first. That is, have working Samsung and OMAP3 code applied in parallel. While Jean-Christophe might cry "code duplication", I learned from OMAP3 boards that is was easier to unify common code _after_ code duplication was done and therefore can be easily identified. Discussing about possible code duplication without being able to see (and test) the code is sometimes a little tricky ;) As mentioned, doing this in several, small steps I feel more comfortable with. If we would have done step (1) already, it's my feeling that we would have reached step 2 or 3 already. But now, we are still discussing about the 'one big perfect patch'. Best regards Dirk >>> I am making this workflow up as I go.. but it seems like the >>> way to resolve this is to work through the new sub-arm repo's >>> >>> #1 should go to u-boot-ti first and then I will will merge it to u-boot-arm >>> Then u-boot-samsung can sync to it and we will all be at a good >>> starting point for #2. >>> Can #1 go now to u-boot-ti ? >>> If there is a merge problem, kick it back to the developer ;) >>> >>> This patch moves the cache support out of A8 and into omap3 which is >>> the correct place for it. >>> >>> I assume the samsung changes are going to go first to u-boot-samsung >>> Correct ? >>> >>> For 2a) runtime cache enabling / disabling >>> New feature I don't think omap needs so it should be at some board or >>> soc level that does not impact omap. >>> >>> For 2b) get_device_type. This is an omap-ism that goes away with #1 >>> >>> The means though that samsung needs its own cache routines. >>> If they are going to do 2a) they will need them anyway. >>> >>> For 3) I don't think omap needs improving at this point. >>> >>> For 4) Lets see how much the samsung cache routines look like the >>> omap once they are done. If it is a lot of cut-n-paste this is >>> not worth arguing about a common routine will be easier to manage. >>> If the cache code looks really different then it can live at the >>> board or soc layer. As long as no one claims the cpu layer at the >>> very least everyone's board will work. >>> >>> >>> Tom >>> >> >> Although I did not understand all of the cache routine.. >> the s5pc100 soc doesn't need v7_flush_dcache_all function >> and other cache routines. >> But the s5pc110 soc needs this function, >> and it works fine without modification. (please see my patch) >> > > Please send me a link to your patch. > > Tom > > >> I think.. v7_flush_decache_all function can share together. >> >> If this function is not moved to soc layer, >> need to remove omap specific codes (checking device type) >> > >> Thanks. >> Minkyu Kang > > ___ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot > http://lists.denx.de/pipermail/u-boot/2009-September/059889.html top of this thread thanks. -- from. prom. www.promsoft.net ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm_cortexa8: support cache flush to other soc
Minkyu Kang wrote: > Tom wrote: >> Dirk Behme wrote: >>> Tom wrote: Minkyu Kang wrote: > Dear Dirk, > > I have lost track of this thread. >>> Yes, and I'm currently trying to get the track back ;) >>> When last I worked this, it seemed like the cache routines were going to be split. 1. generic cache routines 2. legacy soc cache routines. Are the generic cache routines in place and can you use them? Else can you handle your soc specific cache routines as part of a legacy cache routine ? The omap cache routines are dependent on omap rom code and fitting new routines in using the omap specifics may not be the best way to go. >>> I'm sure this is not the perfect way, but it seems to me that >>> splitting all this stuff into several small steps would be the easier >>> for all. E.g. >>> >>> 1) From the previous discussion I think we should apply >>> >>> http://lists.denx.de/pipermail/u-boot/2009-August/058492.html >>> >>> Independent of any discussion if this code is needed at all, if it is >>> generic or not etc. the main advantage I understood is that it frees >>> the way for Samsung. >>> >>> 2a) Then, what I understood from Minkyu, we need an additional patch >>> (discussion?) how to switch CONFIG_L2_OFF from compile time to run >>> time selection (for Samsung's multi board support) >>> >>> 2b) Then, what I understood from Minkyu, we should discuss about >>> removal of get_device_type() function >>> >>> 3) In parallel, we should discuss how to further improve the OMAP3 >>> cache stuff. What might be generic, what not, what isn't necessary etc. >>> >>> 4) Regarding (cache) code duplication, I vote for doing this >>> duplication first. That is, have working Samsung and OMAP3 code >>> applied in parallel. While Jean-Christophe might cry "code >>> duplication", I learned from OMAP3 boards that is was easier to unify >>> common code _after_ code duplication was done and therefore can be >>> easily identified. Discussing about possible code duplication without >>> being able to see (and test) the code is sometimes a little tricky ;) >>> >>> As mentioned, doing this in several, small steps I feel more >>> comfortable with. If we would have done step (1) already, it's my >>> feeling that we would have reached step 2 or 3 already. But now, we >>> are still discussing about the 'one big perfect patch'. >>> >>> Best regards >>> >>> Dirk >>> >>> >> I am making this workflow up as I go.. but it seems like the >> way to resolve this is to work through the new sub-arm repo's >> >> #1 should go to u-boot-ti first and then I will will merge it to u-boot-arm >> Then u-boot-samsung can sync to it and we will all be at a good >> starting point for #2. >> Can #1 go now to u-boot-ti ? >> If there is a merge problem, kick it back to the developer ;) >> >> This patch moves the cache support out of A8 and into omap3 which is >> the correct place for it. >> >> I assume the samsung changes are going to go first to u-boot-samsung >> Correct ? >> >> For 2a) runtime cache enabling / disabling >> New feature I don't think omap needs so it should be at some board or >> soc level that does not impact omap. >> >> For 2b) get_device_type. This is an omap-ism that goes away with #1 >> >> The means though that samsung needs its own cache routines. >> If they are going to do 2a) they will need them anyway. >> >> For 3) I don't think omap needs improving at this point. >> >> For 4) Lets see how much the samsung cache routines look like the >> omap once they are done. If it is a lot of cut-n-paste this is >> not worth arguing about a common routine will be easier to manage. >> If the cache code looks really different then it can live at the >> board or soc layer. As long as no one claims the cpu layer at the >> very least everyone's board will work. >> >> >> Tom >> > > Although I did not understand all of the cache routine.. > the s5pc100 soc doesn't need v7_flush_dcache_all function > and other cache routines. > But the s5pc110 soc needs this function, > and it works fine without modification. (please see my patch) > Please send me a link to your patch. Tom > I think.. v7_flush_decache_all function can share together. > > If this function is not moved to soc layer, > need to remove omap specific codes (checking device type) > > Thanks. > Minkyu Kang ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm_cortexa8: support cache flush to other soc
Tom wrote: > Dirk Behme wrote: >> Tom wrote: >>> Minkyu Kang wrote: Dear Dirk, >>> >>> >>> I have lost track of this thread. >> >> Yes, and I'm currently trying to get the track back ;) >> >>> When last I worked this, it seemed like the cache routines were going to >>> be split. >>> >>> 1. generic cache routines >>> 2. legacy soc cache routines. >>> >>> Are the generic cache routines in place and can you use them? >>> Else can you handle your soc specific cache routines as part of a >>> legacy cache routine ? >>> >>> The omap cache routines are dependent on omap rom code and fitting >>> new routines in using the omap specifics may not be the best way to >>> go. >> >> I'm sure this is not the perfect way, but it seems to me that >> splitting all this stuff into several small steps would be the easier >> for all. E.g. >> >> 1) From the previous discussion I think we should apply >> >> http://lists.denx.de/pipermail/u-boot/2009-August/058492.html >> >> Independent of any discussion if this code is needed at all, if it is >> generic or not etc. the main advantage I understood is that it frees >> the way for Samsung. >> >> 2a) Then, what I understood from Minkyu, we need an additional patch >> (discussion?) how to switch CONFIG_L2_OFF from compile time to run >> time selection (for Samsung's multi board support) >> >> 2b) Then, what I understood from Minkyu, we should discuss about >> removal of get_device_type() function >> >> 3) In parallel, we should discuss how to further improve the OMAP3 >> cache stuff. What might be generic, what not, what isn't necessary etc. >> >> 4) Regarding (cache) code duplication, I vote for doing this >> duplication first. That is, have working Samsung and OMAP3 code >> applied in parallel. While Jean-Christophe might cry "code >> duplication", I learned from OMAP3 boards that is was easier to unify >> common code _after_ code duplication was done and therefore can be >> easily identified. Discussing about possible code duplication without >> being able to see (and test) the code is sometimes a little tricky ;) >> >> As mentioned, doing this in several, small steps I feel more >> comfortable with. If we would have done step (1) already, it's my >> feeling that we would have reached step 2 or 3 already. But now, we >> are still discussing about the 'one big perfect patch'. >> >> Best regards >> >> Dirk >> >> > > I am making this workflow up as I go.. but it seems like the > way to resolve this is to work through the new sub-arm repo's > > #1 should go to u-boot-ti first and then I will will merge it to u-boot-arm > Then u-boot-samsung can sync to it and we will all be at a good > starting point for #2. > Can #1 go now to u-boot-ti ? > If there is a merge problem, kick it back to the developer ;) > > This patch moves the cache support out of A8 and into omap3 which is > the correct place for it. > > I assume the samsung changes are going to go first to u-boot-samsung > Correct ? > > For 2a) runtime cache enabling / disabling > New feature I don't think omap needs so it should be at some board or > soc level that does not impact omap. > > For 2b) get_device_type. This is an omap-ism that goes away with #1 > > The means though that samsung needs its own cache routines. > If they are going to do 2a) they will need them anyway. > > For 3) I don't think omap needs improving at this point. > > For 4) Lets see how much the samsung cache routines look like the > omap once they are done. If it is a lot of cut-n-paste this is > not worth arguing about a common routine will be easier to manage. > If the cache code looks really different then it can live at the > board or soc layer. As long as no one claims the cpu layer at the > very least everyone's board will work. > > > Tom > Although I did not understand all of the cache routine.. the s5pc100 soc doesn't need v7_flush_dcache_all function and other cache routines. But the s5pc110 soc needs this function, and it works fine without modification. (please see my patch) I think.. v7_flush_decache_all function can share together. If this function is not moved to soc layer, need to remove omap specific codes (checking device type) Thanks. Minkyu Kang ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm_cortexa8: support cache flush to other soc
Dirk Behme wrote: > Tom wrote: >> Minkyu Kang wrote: >>> Dear Dirk, >>> >>> >> >> >> I have lost track of this thread. > > Yes, and I'm currently trying to get the track back ;) > >> When last I worked this, it seemed like the cache routines were going to >> be split. >> >> 1. generic cache routines >> 2. legacy soc cache routines. >> >> Are the generic cache routines in place and can you use them? >> Else can you handle your soc specific cache routines as part of a >> legacy cache routine ? >> >> The omap cache routines are dependent on omap rom code and fitting >> new routines in using the omap specifics may not be the best way to >> go. > > I'm sure this is not the perfect way, but it seems to me that splitting > all this stuff into several small steps would be the easier for all. E.g. > > 1) From the previous discussion I think we should apply > > http://lists.denx.de/pipermail/u-boot/2009-August/058492.html > > Independent of any discussion if this code is needed at all, if it is > generic or not etc. the main advantage I understood is that it frees the > way for Samsung. > > 2a) Then, what I understood from Minkyu, we need an additional patch > (discussion?) how to switch CONFIG_L2_OFF from compile time to run time > selection (for Samsung's multi board support) > > 2b) Then, what I understood from Minkyu, we should discuss about removal > of get_device_type() function > > 3) In parallel, we should discuss how to further improve the OMAP3 cache > stuff. What might be generic, what not, what isn't necessary etc. > > 4) Regarding (cache) code duplication, I vote for doing this duplication > first. That is, have working Samsung and OMAP3 code applied in parallel. > While Jean-Christophe might cry "code duplication", I learned from OMAP3 > boards that is was easier to unify common code _after_ code duplication > was done and therefore can be easily identified. Discussing about > possible code duplication without being able to see (and test) the code > is sometimes a little tricky ;) > > As mentioned, doing this in several, small steps I feel more comfortable > with. If we would have done step (1) already, it's my feeling that we > would have reached step 2 or 3 already. But now, we are still discussing > about the 'one big perfect patch'. > > Best regards > > Dirk > > I am making this workflow up as I go.. but it seems like the way to resolve this is to work through the new sub-arm repo's #1 should go to u-boot-ti first and then I will will merge it to u-boot-arm Then u-boot-samsung can sync to it and we will all be at a good starting point for #2. Can #1 go now to u-boot-ti ? If there is a merge problem, kick it back to the developer ;) This patch moves the cache support out of A8 and into omap3 which is the correct place for it. I assume the samsung changes are going to go first to u-boot-samsung Correct ? For 2a) runtime cache enabling / disabling New feature I don't think omap needs so it should be at some board or soc level that does not impact omap. For 2b) get_device_type. This is an omap-ism that goes away with #1 The means though that samsung needs its own cache routines. If they are going to do 2a) they will need them anyway. For 3) I don't think omap needs improving at this point. For 4) Lets see how much the samsung cache routines look like the omap once they are done. If it is a lot of cut-n-paste this is not worth arguing about a common routine will be easier to manage. If the cache code looks really different then it can live at the board or soc layer. As long as no one claims the cpu layer at the very least everyone's board will work. Tom ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm_cortexa8: support cache flush to other soc
Tom wrote: > Minkyu Kang wrote: >> Dear Dirk, >> >> > > > I have lost track of this thread. Yes, and I'm currently trying to get the track back ;) > When last I worked this, it seemed like the cache routines were going to > be split. > > 1. generic cache routines > 2. legacy soc cache routines. > > Are the generic cache routines in place and can you use them? > Else can you handle your soc specific cache routines as part of a > legacy cache routine ? > > The omap cache routines are dependent on omap rom code and fitting > new routines in using the omap specifics may not be the best way to > go. I'm sure this is not the perfect way, but it seems to me that splitting all this stuff into several small steps would be the easier for all. E.g. 1) From the previous discussion I think we should apply http://lists.denx.de/pipermail/u-boot/2009-August/058492.html Independent of any discussion if this code is needed at all, if it is generic or not etc. the main advantage I understood is that it frees the way for Samsung. 2a) Then, what I understood from Minkyu, we need an additional patch (discussion?) how to switch CONFIG_L2_OFF from compile time to run time selection (for Samsung's multi board support) 2b) Then, what I understood from Minkyu, we should discuss about removal of get_device_type() function 3) In parallel, we should discuss how to further improve the OMAP3 cache stuff. What might be generic, what not, what isn't necessary etc. 4) Regarding (cache) code duplication, I vote for doing this duplication first. That is, have working Samsung and OMAP3 code applied in parallel. While Jean-Christophe might cry "code duplication", I learned from OMAP3 boards that is was easier to unify common code _after_ code duplication was done and therefore can be easily identified. Discussing about possible code duplication without being able to see (and test) the code is sometimes a little tricky ;) As mentioned, doing this in several, small steps I feel more comfortable with. If we would have done step (1) already, it's my feeling that we would have reached step 2 or 3 already. But now, we are still discussing about the 'one big perfect patch'. Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm_cortexa8: support cache flush to other soc
Minkyu Kang wrote: > Dear Dirk, > > I have lost track of this thread. When last I worked this, it seemed like the cache routines were going to be split. 1. generic cache routines 2. legacy soc cache routines. Are the generic cache routines in place and can you use them? Else can you handle your soc specific cache routines as part of a legacy cache routine ? The omap cache routines are dependent on omap rom code and fitting new routines in using the omap specifics may not be the best way to go. Tom > > Thanks :) > Minkyu Kang > > > > > > ___ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm_cortexa8: support cache flush to other soc
Dear Dirk, >>> I have two request. >> >> 1. I want to replace CONFIG_L2_OFF define to other function >> >> for example.. >> >>if (need_cache_flush()) { /* or !l2_off() */ >>/* turn off L2 cache */ >>l2_cache_disable(); >>/* invalidate L2 cache also */ >>v7_flush_dcache_all(get_device_type()); >> >>i = 0; >>/* mem barrier to sync up things */ >>asm("mcr p15, 0, %0, c7, c10, 4": :"r"(i)); >> >>l2_cache_enable(); >>} >> >> >> 2. I don't want to use get_device_type() function >> (just call like this: v7_flush_decahe_all() ) >> >> How you think? >> > > I wonder if these two requests are > > - nice to have topics? Or > > - required changes? > request no.1 is partly "required" but not now, it is required for supporting s5pc110 soc. because of we want to use same binary at runtime, so have to avoid ifdef of cause, we can use other ways.. but i think it's good way that is removing ifdef request no.2 is just "request" If you don't want to accept this, we can use the function. but I think get_device_type() does not fit with cpu common file > > And if these are required changes, if > > - these can go on top of > > http://lists.denx.de/pipermail/u-boot/2009-August/058492.html > > so that we can apply above patch, you are able to go on to bring Samsung > into mainline? And in parallel we discuss/change above topics? Or > > - we have to rewrite > > http://lists.denx.de/pipermail/u-boot/2009-August/058492.html > > what will stall Samsungs mainline integration further? > > Best regards > > Dirk > > As a result, you can apply the patch. but I hope we will process my request soon. Thanks :) Minkyu Kang -- from. prom. www.promsoft.net ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm_cortexa8: support cache flush to other soc
Dear Minkyu Kang, Minkyu Kang wrote: > Dear Dirk, > > 2009/9/5 Dirk Behme > >> Minkyu Kang wrote: >> >>> Dear, Dirk >>> >>> 2009/9/4 Dirk Behme >>> >>> Kyungmin Park wrote: > On Fri, Sep 4, 2009 at 7:45 PM, Dirk Behme > wrote: > Kyungmin Park wrote: > ... > + if (get_device_type() != 0xC100) { Hmm, what is this "0xC100" ? >>> Now we got two cpu, s5pc100 and s5pc110. In case of s5pc100 we don't >>> need to turn off l2 cache. but s5pc110 needs it. >>> So first check the device type, actually cpu type. then determine turn >>> off l2 cache or not. >>> >> "0xC100" is the device type of s5pc100 then? So it should be >> >> if (get_device_type() != S5PC100_DEVICE) >> >> ? I hear some people crying "please use macro" ;) >> > Agreed. DONT_NEED_CACHE_FLUSH? > > But I don't like this selection here. When we get additional similar > SoCs, > we will end with something like >> if (get_device_type() != 0xC100) || >> (get_device_type() != FOO) || >> (get_device_type() != BAR)) || >> ... { >> >> modifying each time cpu/arm_cortexa8/cpu.c. >> >> I would like more that we are able to compile the functionality based >> on >> > the > config file we use for compilation. E.g. provide emtpy > l2_cache_disable(); > function for SoCs that don't need it, but have functionality behind it > where > needed. >> With above patch, this would then become something like >> >> cpu/arm_cortexa8/s5pcxxx/dcache.S >> >> -> Implements invalidate_dcache() (or implement a Cortex A8 generic one >> > in > cpu/arm_cortexa8/cache.S) >> cpu/arm_cortexa8/s5pcxxx/cache_110.S >> >> -> Implements l2_cache_enable()/disable() >> >> cpu/arm_cortexa8/s5pcxxx/cache_100.S >> >> -> Implements *empty* l2_cache_enable()/disable() >> >> In cpu/arm_cortexa8/s5pcxxx/Makefile you then could have >> >> SOBJS-y += dcache.o >> SOBJS-$(CONFIG_S5PC100) += cache_100.o >> SOBJS-$(CONFIG_S5PC110) += cache_110.o >> >> What do you think about this? >> >> Basically agreed, of course we can think weak attribute but now we > have to support both cpu simultaneously. > with this reason. we check the device_type at here. > What's about having this check in SoC specific code instead of Cortex A8 generic code, then? E.g apply patch http://lists.denx.de/pipermail/u-boot/2009-August/058492.html and then create cpu/arm_cortexa8/s5pcxxx/cache.S with invalidate_dcache() { if (get_device_type() == S5PC100_DEVICE) return(); ... l2_cache_enable() { if (get_device_type() == S5PC100_DEVICE) return(); ... etc. That is, have the SoC dependent part in SoC specific directory/file. Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot >>> I know about the discussion of this issue between you and Jean-Christophe. >>> but it is gone without resolving. >>> So, I want to make issue again. >>> anyway,, >>> >>> Actually, we don't need the function of get_device_type() >>> I think that function is omap specific function.. isn't it? >>> but.. because of current code already use that function, I had to use that >>> function >>> If you have plan to move the cache routines into SoC, >>> I think you can remove the argument for device_type. (check device type in >>> omap3's cache routines) >>> >>> And I want to remove CONFIG_L2_OFF also. >>> We can know this through device type or soc type. >>> How about make new function? >>> e.g l2_off() or need_cache_flush() etc, >>> >>> Please rework for removing dependency of omap3 soc first. >>> >> Just to clarify: It's my understanding that this is already done by >> >> http://lists.denx.de/pipermail/u-boot/2009-August/058492.html >> >> Do you agree? That is, when this patch is applied, then Samsung can go on. >> Correct? >> >> If not correct, what is missing in above patch? >> >> Best regards >> >> Dirk >> >> >> > I have two request. > > 1. I want to replace CONFIG_L2_OFF define to other function > > for example.. > > if (need_cache_flush()) { /* or !l2_off() */ > /* turn off L2 cache */ > l2_cache_disable(); > /* invalidate L2 cache also */ > v7_flush_dcache_all(get_device_type()); > > i = 0; > /* mem barrier to sync up things */ > asm("mcr p15, 0, %0, c7, c10, 4": :"r"(i)); > > l2_cache_enable(); > } > > > 2. I don't want to use get_device_type() function > (just call like this: v7_flush_decahe_all() ) > > How you think? I wonder if these two requests are - nice to have topics? Or - requ
Re: [U-Boot] [PATCH] arm_cortexa8: support cache flush to other soc
Dear Dirk, 2009/9/5 Dirk Behme > Minkyu Kang wrote: > >> Dear, Dirk >> >> 2009/9/4 Dirk Behme >> >> Kyungmin Park wrote: >>> On Fri, Sep 4, 2009 at 7:45 PM, Dirk Behme >>> wrote: >>> Kyungmin Park wrote: > ... >>> + if (get_device_type() != 0xC100) { >>> Hmm, what is this "0xC100" ? >>> >> Now we got two cpu, s5pc100 and s5pc110. In case of s5pc100 we don't >> need to turn off l2 cache. but s5pc110 needs it. >> So first check the device type, actually cpu type. then determine turn >> off l2 cache or not. >> > "0xC100" is the device type of s5pc100 then? So it should be > > if (get_device_type() != S5PC100_DEVICE) > > ? I hear some people crying "please use macro" ;) > Agreed. DONT_NEED_CACHE_FLUSH? But I don't like this selection here. When we get additional similar > SoCs, >>> we will end with something like > > if (get_device_type() != 0xC100) || > (get_device_type() != FOO) || > (get_device_type() != BAR)) || > ... { > > modifying each time cpu/arm_cortexa8/cpu.c. > > I would like more that we are able to compile the functionality based > on > the >>> config file we use for compilation. E.g. provide emtpy > l2_cache_disable(); >>> function for SoCs that don't need it, but have functionality behind it > where >>> needed. > > With above patch, this would then become something like > > cpu/arm_cortexa8/s5pcxxx/dcache.S > > -> Implements invalidate_dcache() (or implement a Cortex A8 generic one > in >>> cpu/arm_cortexa8/cache.S) > > cpu/arm_cortexa8/s5pcxxx/cache_110.S > > -> Implements l2_cache_enable()/disable() > > cpu/arm_cortexa8/s5pcxxx/cache_100.S > > -> Implements *empty* l2_cache_enable()/disable() > > In cpu/arm_cortexa8/s5pcxxx/Makefile you then could have > > SOBJS-y += dcache.o > SOBJS-$(CONFIG_S5PC100) += cache_100.o > SOBJS-$(CONFIG_S5PC110) += cache_110.o > > What do you think about this? > > Basically agreed, of course we can think weak attribute but now we have to support both cpu simultaneously. with this reason. we check the device_type at here. >>> What's about having this check in SoC specific code instead of Cortex >>> A8 generic code, then? >>> >>> E.g apply patch >>> >>> http://lists.denx.de/pipermail/u-boot/2009-August/058492.html >>> >>> and then create >>> >>> cpu/arm_cortexa8/s5pcxxx/cache.S >>> >>> with >>> >>> invalidate_dcache() { >>> if (get_device_type() == S5PC100_DEVICE) >>>return(); >>> ... >>> >>> l2_cache_enable() { >>> if (get_device_type() == S5PC100_DEVICE) >>>return(); >>> ... >>> >>> etc. >>> >>> That is, have the SoC dependent part in SoC specific directory/file. >>> >>> Best regards >>> >>> Dirk >>> >>> >>> >>> ___ >>> U-Boot mailing list >>> U-Boot@lists.denx.de >>> http://lists.denx.de/mailman/listinfo/u-boot >>> >>> >> I know about the discussion of this issue between you and Jean-Christophe. >> but it is gone without resolving. >> So, I want to make issue again. >> anyway,, >> >> Actually, we don't need the function of get_device_type() >> I think that function is omap specific function.. isn't it? >> but.. because of current code already use that function, I had to use that >> function >> If you have plan to move the cache routines into SoC, >> I think you can remove the argument for device_type. (check device type in >> omap3's cache routines) >> >> And I want to remove CONFIG_L2_OFF also. >> We can know this through device type or soc type. >> How about make new function? >> e.g l2_off() or need_cache_flush() etc, >> >> Please rework for removing dependency of omap3 soc first. >> > > Just to clarify: It's my understanding that this is already done by > > http://lists.denx.de/pipermail/u-boot/2009-August/058492.html > > Do you agree? That is, when this patch is applied, then Samsung can go on. > Correct? > > If not correct, what is missing in above patch? > > Best regards > > Dirk > > > I have two request. 1. I want to replace CONFIG_L2_OFF define to other function for example.. if (need_cache_flush()) { /* or !l2_off() */ /* turn off L2 cache */ l2_cache_disable(); /* invalidate L2 cache also */ v7_flush_dcache_all(get_device_type()); i = 0; /* mem barrier to sync up things */ asm("mcr p15, 0, %0, c7, c10, 4": :"r"(i)); l2_cache_enable(); } 2. I don't want to use get_device_type() function (just call like this: v7_flush_decahe_all() ) How you think? thanks Minkyu Kang -- from. prom. www.promsoft.net ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm_cortexa8: support cache flush to other soc
On 12:45 Fri 04 Sep , Dirk Behme wrote: > Kyungmin Park wrote: > > Hi, > > > > As he goes to home, I reply it instead. > > Nice weekend then :) > > > On Fri, Sep 4, 2009 at 5:43 PM, Dirk Behme wrote: > >> Dear Minkyu Kang, > >> > >> Minkyu Kang wrote: > >>> Current code is supported only omap3 soc. > >>> this patch will support s5pc1xx(s5pc100 and s5pc110) soc also. > >> Thanks for this patch! > >> > >> How is this patch related to > >> > >> http://lists.denx.de/pipermail/u-boot/2009-August/058492.html > >> > > > > It's not good idea to move invalidate_cache to omap directory. we need it. > > Well, yes and no ;) > > Most probably you (== Samsung) can't use the invalidate_dcache version > we move in above patch to omap directory, because the version we move > above is OMAP3 specific (it has calls to OMAP3 ROM code). So no, it's > a good idea to move OMAP3 specific code to omap directory. It's certainly not as the OMAP3 does not need to call the ROM code so we can share the same for for S3P and omap3 > > But yes, you might need DCache flush (*). So the idea of above patch > was to have your own (or generic) implementation, but let OMAP3 use > the custom one where needed. no need for omap3 > > (*) Do you really need DCache flush? It always was Jean-Christophe's > argument that U-Boot doesn't use any DCache at all. It's true but U-Boot is not necessarely the first stage loader so yes we will clean all cache > > "0xC100" is the device type of s5pc100 then? So it should be > > if (get_device_type() != S5PC100_DEVICE) > > ? I hear some people crying "please use macro" ;) > > But I don't like this selection here. When we get additional similar > SoCs, we will end with something like > > if (get_device_type() != 0xC100) || >(get_device_type() != FOO) || >(get_device_type() != BAR)) || >... { > > modifying each time cpu/arm_cortexa8/cpu.c. > > I would like more that we are able to compile the functionality based > on the config file we use for compilation. E.g. provide emtpy > l2_cache_disable(); function for SoCs that don't need it, but have > functionality behind it where needed. multiple soc support will clearly simplify board support and improve code testing but it's need to have the possiblity to be disable as the compile to minimize the size impact Best Regards, n J ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm_cortexa8: support cache flush to other soc
Minkyu Kang wrote: > Dear, Dirk > > 2009/9/4 Dirk Behme > >> Kyungmin Park wrote: >>> On Fri, Sep 4, 2009 at 7:45 PM, Dirk Behme >> wrote: Kyungmin Park wrote: >> ... >>> + if (get_device_type() != 0xC100) { >> Hmm, what is this "0xC100" ? > Now we got two cpu, s5pc100 and s5pc110. In case of s5pc100 we don't > need to turn off l2 cache. but s5pc110 needs it. > So first check the device type, actually cpu type. then determine turn > off l2 cache or not. "0xC100" is the device type of s5pc100 then? So it should be if (get_device_type() != S5PC100_DEVICE) ? I hear some people crying "please use macro" ;) >>> Agreed. DONT_NEED_CACHE_FLUSH? >>> But I don't like this selection here. When we get additional similar >> SoCs, we will end with something like if (get_device_type() != 0xC100) || (get_device_type() != FOO) || (get_device_type() != BAR)) || ... { modifying each time cpu/arm_cortexa8/cpu.c. I would like more that we are able to compile the functionality based on >> the config file we use for compilation. E.g. provide emtpy >> l2_cache_disable(); function for SoCs that don't need it, but have functionality behind it >> where needed. With above patch, this would then become something like cpu/arm_cortexa8/s5pcxxx/dcache.S -> Implements invalidate_dcache() (or implement a Cortex A8 generic one >> in cpu/arm_cortexa8/cache.S) cpu/arm_cortexa8/s5pcxxx/cache_110.S -> Implements l2_cache_enable()/disable() cpu/arm_cortexa8/s5pcxxx/cache_100.S -> Implements *empty* l2_cache_enable()/disable() In cpu/arm_cortexa8/s5pcxxx/Makefile you then could have SOBJS-y += dcache.o SOBJS-$(CONFIG_S5PC100) += cache_100.o SOBJS-$(CONFIG_S5PC110) += cache_110.o What do you think about this? >>> Basically agreed, of course we can think weak attribute but now we >>> have to support both cpu simultaneously. >>> with this reason. we check the device_type at here. >> What's about having this check in SoC specific code instead of Cortex >> A8 generic code, then? >> >> E.g apply patch >> >> http://lists.denx.de/pipermail/u-boot/2009-August/058492.html >> >> and then create >> >> cpu/arm_cortexa8/s5pcxxx/cache.S >> >> with >> >> invalidate_dcache() { >> if (get_device_type() == S5PC100_DEVICE) >> return(); >> ... >> >> l2_cache_enable() { >>if (get_device_type() == S5PC100_DEVICE) >> return(); >> ... >> >> etc. >> >> That is, have the SoC dependent part in SoC specific directory/file. >> >> Best regards >> >> Dirk >> >> >> >> ___ >> U-Boot mailing list >> U-Boot@lists.denx.de >> http://lists.denx.de/mailman/listinfo/u-boot >> > > I know about the discussion of this issue between you and Jean-Christophe. > but it is gone without resolving. > So, I want to make issue again. > anyway,, > > Actually, we don't need the function of get_device_type() > I think that function is omap specific function.. isn't it? > but.. because of current code already use that function, I had to use that > function > If you have plan to move the cache routines into SoC, > I think you can remove the argument for device_type. (check device type in > omap3's cache routines) > > And I want to remove CONFIG_L2_OFF also. > We can know this through device type or soc type. > How about make new function? > e.g l2_off() or need_cache_flush() etc, > > Please rework for removing dependency of omap3 soc first. Just to clarify: It's my understanding that this is already done by http://lists.denx.de/pipermail/u-boot/2009-August/058492.html Do you agree? That is, when this patch is applied, then Samsung can go on. Correct? If not correct, what is missing in above patch? Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm_cortexa8: support cache flush to other soc
Dear, Dirk 2009/9/4 Dirk Behme > Kyungmin Park wrote: > > On Fri, Sep 4, 2009 at 7:45 PM, Dirk Behme > wrote: > >> Kyungmin Park wrote: > ... > > + if (get_device_type() != 0xC100) { > Hmm, what is this "0xC100" ? > >>> Now we got two cpu, s5pc100 and s5pc110. In case of s5pc100 we don't > >>> need to turn off l2 cache. but s5pc110 needs it. > >>> So first check the device type, actually cpu type. then determine turn > >>> off l2 cache or not. > >> "0xC100" is the device type of s5pc100 then? So it should be > >> > >> if (get_device_type() != S5PC100_DEVICE) > >> > >> ? I hear some people crying "please use macro" ;) > > > > Agreed. DONT_NEED_CACHE_FLUSH? > > > >> But I don't like this selection here. When we get additional similar > SoCs, > >> we will end with something like > >> > >> if (get_device_type() != 0xC100) || > >> (get_device_type() != FOO) || > >> (get_device_type() != BAR)) || > >> ... { > >> > >> modifying each time cpu/arm_cortexa8/cpu.c. > >> > >> I would like more that we are able to compile the functionality based on > the > >> config file we use for compilation. E.g. provide emtpy > l2_cache_disable(); > >> function for SoCs that don't need it, but have functionality behind it > where > >> needed. > >> > >> With above patch, this would then become something like > >> > >> cpu/arm_cortexa8/s5pcxxx/dcache.S > >> > >> -> Implements invalidate_dcache() (or implement a Cortex A8 generic one > in > >> cpu/arm_cortexa8/cache.S) > >> > >> cpu/arm_cortexa8/s5pcxxx/cache_110.S > >> > >> -> Implements l2_cache_enable()/disable() > >> > >> cpu/arm_cortexa8/s5pcxxx/cache_100.S > >> > >> -> Implements *empty* l2_cache_enable()/disable() > >> > >> In cpu/arm_cortexa8/s5pcxxx/Makefile you then could have > >> > >> SOBJS-y += dcache.o > >> SOBJS-$(CONFIG_S5PC100) += cache_100.o > >> SOBJS-$(CONFIG_S5PC110) += cache_110.o > >> > >> What do you think about this? > >> > > > > Basically agreed, of course we can think weak attribute but now we > > have to support both cpu simultaneously. > > with this reason. we check the device_type at here. > > What's about having this check in SoC specific code instead of Cortex > A8 generic code, then? > > E.g apply patch > > http://lists.denx.de/pipermail/u-boot/2009-August/058492.html > > and then create > > cpu/arm_cortexa8/s5pcxxx/cache.S > > with > > invalidate_dcache() { > if (get_device_type() == S5PC100_DEVICE) > return(); > ... > > l2_cache_enable() { >if (get_device_type() == S5PC100_DEVICE) > return(); > ... > > etc. > > That is, have the SoC dependent part in SoC specific directory/file. > > Best regards > > Dirk > > > > ___ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot > I know about the discussion of this issue between you and Jean-Christophe. but it is gone without resolving. So, I want to make issue again. anyway,, Actually, we don't need the function of get_device_type() I think that function is omap specific function.. isn't it? but.. because of current code already use that function, I had to use that function If you have plan to move the cache routines into SoC, I think you can remove the argument for device_type. (check device type in omap3's cache routines) And I want to remove CONFIG_L2_OFF also. We can know this through device type or soc type. How about make new function? e.g l2_off() or need_cache_flush() etc, Please rework for removing dependency of omap3 soc first. And then, I'll make new patch for s5pc1xx soc. Many thanks :) Minkyu Kang. -- from. prom. www.promsoft.net ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm_cortexa8: support cache flush to other soc
Dear Kyungmin Park, In message <9c9fda240909040411w468baddv9cdd59fafeab2...@mail.gmail.com> you wrote: > > > Do you use the very same U-Boot image on both SoCs? > > Yes. One U-Boot image support 2 different CPU and 7 different boards. > We don't want to make a u-boot for each board. Ah, this explains a lot. Thanks for this explanation. Um... do you have any plans how to handle this in a clean way that will scale for additional SoCs and boards as well, and that eventually can be used for other architectures, too? I guess you will have to address the same issues in your Linux ports? Do you consider using the device tree to implement the capability to adjust the software configuration to the specifics of the current hardware? I think this would be really beneficial both for U-Boot and Linux. I have to admit that I'm pretty much stuck between a rock and a hard place. I highly appreciate the fact that Samsung starts contributing code to Linux and now also U-Boot mainline trees,. and as such I want to encourage you all as much as possible, and not put any road blocks in your way when it can be avoided. I already hesitated when I rejected all these base address plus offset things and demanded these to be changed to C structs. I am well aware that is causes a lot of effort on your side without any immediately visible benefit (your code is already running, so why change it?). On the other hand, I have to keep an eye at the overall code quality, and to make sure the coderemains maintenable. >From the patches I've seen so far I fear that we weill quickly see the code copmplexity explode when it comes to handling different SoC and board combinations within a single U-Boot image. I would like to point out that we should not go the way that is currently being used in the ARM Linux kernel. We will not duplicate the MACH_ID concepts used there and the resulting code methods. Instead, when we need to add such flexible configuration, we will go the device tree way. I hope such comment still comes in time to help you taking the right turns. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de panic: kernel trap (ignored) ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm_cortexa8: support cache flush to other soc
Kyungmin Park wrote: > On Fri, Sep 4, 2009 at 7:45 PM, Dirk Behme wrote: >> Kyungmin Park wrote: ... > + if (get_device_type() != 0xC100) { Hmm, what is this "0xC100" ? >>> Now we got two cpu, s5pc100 and s5pc110. In case of s5pc100 we don't >>> need to turn off l2 cache. but s5pc110 needs it. >>> So first check the device type, actually cpu type. then determine turn >>> off l2 cache or not. >> "0xC100" is the device type of s5pc100 then? So it should be >> >> if (get_device_type() != S5PC100_DEVICE) >> >> ? I hear some people crying "please use macro" ;) > > Agreed. DONT_NEED_CACHE_FLUSH? > >> But I don't like this selection here. When we get additional similar SoCs, >> we will end with something like >> >> if (get_device_type() != 0xC100) || >> (get_device_type() != FOO) || >> (get_device_type() != BAR)) || >> ... { >> >> modifying each time cpu/arm_cortexa8/cpu.c. >> >> I would like more that we are able to compile the functionality based on the >> config file we use for compilation. E.g. provide emtpy l2_cache_disable(); >> function for SoCs that don't need it, but have functionality behind it where >> needed. >> >> With above patch, this would then become something like >> >> cpu/arm_cortexa8/s5pcxxx/dcache.S >> >> -> Implements invalidate_dcache() (or implement a Cortex A8 generic one in >> cpu/arm_cortexa8/cache.S) >> >> cpu/arm_cortexa8/s5pcxxx/cache_110.S >> >> -> Implements l2_cache_enable()/disable() >> >> cpu/arm_cortexa8/s5pcxxx/cache_100.S >> >> -> Implements *empty* l2_cache_enable()/disable() >> >> In cpu/arm_cortexa8/s5pcxxx/Makefile you then could have >> >> SOBJS-y += dcache.o >> SOBJS-$(CONFIG_S5PC100) += cache_100.o >> SOBJS-$(CONFIG_S5PC110) += cache_110.o >> >> What do you think about this? >> > > Basically agreed, of course we can think weak attribute but now we > have to support both cpu simultaneously. > with this reason. we check the device_type at here. What's about having this check in SoC specific code instead of Cortex A8 generic code, then? E.g apply patch http://lists.denx.de/pipermail/u-boot/2009-August/058492.html and then create cpu/arm_cortexa8/s5pcxxx/cache.S with invalidate_dcache() { if (get_device_type() == S5PC100_DEVICE) return(); ... l2_cache_enable() { if (get_device_type() == S5PC100_DEVICE) return(); ... etc. That is, have the SoC dependent part in SoC specific directory/file. Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm_cortexa8: support cache flush to other soc
On Fri, Sep 4, 2009 at 8:06 PM, Wolfgang Denk wrote: > Dear Kyungmin Park, > > In message <9c9fda240909040234m4fdd7466ybb38d0d0618cd...@mail.gmail.com> you > wrote: >> > ... >> >> #ifndef CONFIG_L2_OFF >> >> - /* turn off L2 cache */ >> >> - l2_cache_disable(); >> >> - /* invalidate L2 cache also */ >> >> - v7_flush_dcache_all(get_device_type()); >> >> -#endif >> >> - i = 0; >> >> - /* mem barrier to sync up things */ >> >> - asm("mcr p15, 0, %0, c7, c10, 4": :"r"(i)); >> >> + if (get_device_type() != 0xC100) { >> > >> > Hmm, what is this "0xC100" ? >> >> Now we got two cpu, s5pc100 and s5pc110. In case of s5pc100 we don't >> need to turn off l2 cache. but s5pc110 needs it. >> So first check the device type, actually cpu type. then determine turn >> off l2 cache or not. > > Well, this definitely needs a comment, doesn;t it? > > And I really dislike such board-specific parts in common code. If what > you want to check is the CPU type, then isn;t there a more direct way? > > And do we need a runtime check here? I think that decision can be made > at compile time, right? To support two different CPU. If it determined at compile time. we can't support different CPUs > > > ... >> maybe not. now we only tested the smdkc100 but actual use is internal >> board for s5pc100 & s5pc110. > > Do you use the very same U-Boot image on both SoCs? Yes. One U-Boot image support 2 different CPU and 7 different boards. We don't want to make a u-boot for each board. Thank you, Kyungmin Park ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm_cortexa8: support cache flush to other soc
Dear Kyungmin Park, In message <9c9fda240909040234m4fdd7466ybb38d0d0618cd...@mail.gmail.com> you wrote: > ... > >> #ifndef CONFIG_L2_OFF > >> - /* turn off L2 cache */ > >> - l2_cache_disable(); > >> - /* invalidate L2 cache also */ > >> - v7_flush_dcache_all(get_device_type()); > >> -#endif > >> - i = 0; > >> - /* mem barrier to sync up things */ > >> - asm("mcr p15, 0, %0, c7, c10, 4": :"r"(i)); > >> + if (get_device_type() != 0xC100) { > > > > Hmm, what is this "0xC100" ? > > Now we got two cpu, s5pc100 and s5pc110. In case of s5pc100 we don't > need to turn off l2 cache. but s5pc110 needs it. > So first check the device type, actually cpu type. then determine turn > off l2 cache or not. Well, this definitely needs a comment, doesn;t it? And I really dislike such board-specific parts in common code. If what you want to check is the CPU type, then isn;t there a more direct way? And do we need a runtime check here? I think that decision can be made at compile time, right? ... > maybe not. now we only tested the smdkc100 but actual use is internal > board for s5pc100 & s5pc110. Do you use the very same U-Boot image on both SoCs? Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Real Programmers always confuse Christmas and Halloween because OCT 31 == DEC 25 ! - Andrew Rutherford (andr...@ucs.adelaide.edu.au) ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm_cortexa8: support cache flush to other soc
On Fri, Sep 4, 2009 at 7:45 PM, Dirk Behme wrote: > Kyungmin Park wrote: >> >> Hi, >> >> As he goes to home, I reply it instead. > > Nice weekend then :) > >> On Fri, Sep 4, 2009 at 5:43 PM, Dirk Behme >> wrote: >>> >>> Dear Minkyu Kang, >>> >>> Minkyu Kang wrote: Current code is supported only omap3 soc. this patch will support s5pc1xx(s5pc100 and s5pc110) soc also. >>> >>> Thanks for this patch! >>> >>> How is this patch related to >>> >>> http://lists.denx.de/pipermail/u-boot/2009-August/058492.html >>> >> >> It's not good idea to move invalidate_cache to omap directory. we need it. > > Well, yes and no ;) > > Most probably you (== Samsung) can't use the invalidate_dcache version we > move in above patch to omap directory, because the version we move above is > OMAP3 specific (it has calls to OMAP3 ROM code). So no, it's a good idea to > move OMAP3 specific code to omap directory. > > But yes, you might need DCache flush (*). So the idea of above patch was to > have your own (or generic) implementation, but let OMAP3 use the custom one > where needed. > > (*) Do you really need DCache flush? It always was Jean-Christophe's > argument that U-Boot doesn't use any DCache at all. Yes, We really want it. At first development time, we don't know why the kernel is boot. almost same as s5pc100 but s5pc110 required cache flush. > Signed-off-by: Minkyu Kang --- cpu/arm_cortexa8/cpu.c | 24 +++- 1 files changed, 11 insertions(+), 13 deletions(-) diff --git a/cpu/arm_cortexa8/cpu.c b/cpu/arm_cortexa8/cpu.c index 5a5981e..3d430b1 100644 --- a/cpu/arm_cortexa8/cpu.c +++ b/cpu/arm_cortexa8/cpu.c @@ -35,9 +35,6 @@ #include #include #include -#ifndef CONFIG_L2_OFF -#include -#endif static void cache_flush(void); @@ -61,17 +58,18 @@ int cleanup_before_linux(void) cache_flush(); #ifndef CONFIG_L2_OFF - /* turn off L2 cache */ - l2_cache_disable(); - /* invalidate L2 cache also */ - v7_flush_dcache_all(get_device_type()); -#endif - i = 0; - /* mem barrier to sync up things */ - asm("mcr p15, 0, %0, c7, c10, 4": :"r"(i)); + if (get_device_type() != 0xC100) { >>> >>> Hmm, what is this "0xC100" ? >> >> Now we got two cpu, s5pc100 and s5pc110. In case of s5pc100 we don't >> need to turn off l2 cache. but s5pc110 needs it. >> So first check the device type, actually cpu type. then determine turn >> off l2 cache or not. > > "0xC100" is the device type of s5pc100 then? So it should be > > if (get_device_type() != S5PC100_DEVICE) > > ? I hear some people crying "please use macro" ;) Agreed. DONT_NEED_CACHE_FLUSH? > > But I don't like this selection here. When we get additional similar SoCs, > we will end with something like > > if (get_device_type() != 0xC100) || > (get_device_type() != FOO) || > (get_device_type() != BAR)) || > ... { > > modifying each time cpu/arm_cortexa8/cpu.c. > > I would like more that we are able to compile the functionality based on the > config file we use for compilation. E.g. provide emtpy l2_cache_disable(); > function for SoCs that don't need it, but have functionality behind it where > needed. > > With above patch, this would then become something like > > cpu/arm_cortexa8/s5pcxxx/dcache.S > > -> Implements invalidate_dcache() (or implement a Cortex A8 generic one in > cpu/arm_cortexa8/cache.S) > > cpu/arm_cortexa8/s5pcxxx/cache_110.S > > -> Implements l2_cache_enable()/disable() > > cpu/arm_cortexa8/s5pcxxx/cache_100.S > > -> Implements *empty* l2_cache_enable()/disable() > > In cpu/arm_cortexa8/s5pcxxx/Makefile you then could have > > SOBJS-y += dcache.o > SOBJS-$(CONFIG_S5PC100) += cache_100.o > SOBJS-$(CONFIG_S5PC110) += cache_110.o > > What do you think about this? > Basically agreed, of course we can think weak attribute but now we have to support both cpu simultaneously. with this reason. we check the device_type at here. Thank you, Kyungmin Park ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm_cortexa8: support cache flush to other soc
Kyungmin Park wrote: > Hi, > > As he goes to home, I reply it instead. Nice weekend then :) > On Fri, Sep 4, 2009 at 5:43 PM, Dirk Behme wrote: >> Dear Minkyu Kang, >> >> Minkyu Kang wrote: >>> Current code is supported only omap3 soc. >>> this patch will support s5pc1xx(s5pc100 and s5pc110) soc also. >> Thanks for this patch! >> >> How is this patch related to >> >> http://lists.denx.de/pipermail/u-boot/2009-August/058492.html >> > > It's not good idea to move invalidate_cache to omap directory. we need it. Well, yes and no ;) Most probably you (== Samsung) can't use the invalidate_dcache version we move in above patch to omap directory, because the version we move above is OMAP3 specific (it has calls to OMAP3 ROM code). So no, it's a good idea to move OMAP3 specific code to omap directory. But yes, you might need DCache flush (*). So the idea of above patch was to have your own (or generic) implementation, but let OMAP3 use the custom one where needed. (*) Do you really need DCache flush? It always was Jean-Christophe's argument that U-Boot doesn't use any DCache at all. >>> Signed-off-by: Minkyu Kang >>> --- >>> cpu/arm_cortexa8/cpu.c | 24 +++- >>> 1 files changed, 11 insertions(+), 13 deletions(-) >>> >>> diff --git a/cpu/arm_cortexa8/cpu.c b/cpu/arm_cortexa8/cpu.c >>> index 5a5981e..3d430b1 100644 >>> --- a/cpu/arm_cortexa8/cpu.c >>> +++ b/cpu/arm_cortexa8/cpu.c >>> @@ -35,9 +35,6 @@ >>> #include >>> #include >>> #include >>> -#ifndef CONFIG_L2_OFF >>> -#include >>> -#endif >>> >>> static void cache_flush(void); >>> >>> @@ -61,17 +58,18 @@ int cleanup_before_linux(void) >>> cache_flush(); >>> >>> #ifndef CONFIG_L2_OFF >>> - /* turn off L2 cache */ >>> - l2_cache_disable(); >>> - /* invalidate L2 cache also */ >>> - v7_flush_dcache_all(get_device_type()); >>> -#endif >>> - i = 0; >>> - /* mem barrier to sync up things */ >>> - asm("mcr p15, 0, %0, c7, c10, 4": :"r"(i)); >>> + if (get_device_type() != 0xC100) { >> Hmm, what is this "0xC100" ? > > Now we got two cpu, s5pc100 and s5pc110. In case of s5pc100 we don't > need to turn off l2 cache. but s5pc110 needs it. > So first check the device type, actually cpu type. then determine turn > off l2 cache or not. "0xC100" is the device type of s5pc100 then? So it should be if (get_device_type() != S5PC100_DEVICE) ? I hear some people crying "please use macro" ;) But I don't like this selection here. When we get additional similar SoCs, we will end with something like if (get_device_type() != 0xC100) || (get_device_type() != FOO) || (get_device_type() != BAR)) || ... { modifying each time cpu/arm_cortexa8/cpu.c. I would like more that we are able to compile the functionality based on the config file we use for compilation. E.g. provide emtpy l2_cache_disable(); function for SoCs that don't need it, but have functionality behind it where needed. With above patch, this would then become something like cpu/arm_cortexa8/s5pcxxx/dcache.S -> Implements invalidate_dcache() (or implement a Cortex A8 generic one in cpu/arm_cortexa8/cache.S) cpu/arm_cortexa8/s5pcxxx/cache_110.S -> Implements l2_cache_enable()/disable() cpu/arm_cortexa8/s5pcxxx/cache_100.S -> Implements *empty* l2_cache_enable()/disable() In cpu/arm_cortexa8/s5pcxxx/Makefile you then could have SOBJS-y += dcache.o SOBJS-$(CONFIG_S5PC100) += cache_100.o SOBJS-$(CONFIG_S5PC110) += cache_110.o What do you think about this? >>> + /* turn off L2 cache */ >>> + l2_cache_disable(); >>> + /* invalidate L2 cache also */ >>> + v7_flush_dcache_all(get_device_type()); >>> >>> -#ifndef CONFIG_L2_OFF >>> - l2_cache_enable(); >>> + i = 0; >>> + /* mem barrier to sync up things */ >>> + asm("mcr p15, 0, %0, c7, c10, 4": :"r"(i)); >>> + >>> + l2_cache_enable(); >>> + } >>> #endif >> What's about the order of #ifndef CONFIG_L2_OFF? >> >> While we had before >> >> >> #ifndef CONFIG_L2_OFF >>/* turn off L2 cache */ >>l2_cache_disable(); >>/* invalidate L2 cache also */ >>v7_flush_dcache_all(get_device_type()); >> #endif >>i = 0; >>/* mem barrier to sync up things */ >>asm("mcr p15, 0, %0, c7, c10, 4": :"r"(i)); >> >> #ifndef CONFIG_L2_OFF >>l2_cache_enable(); >> #endif >> >> >> after this patch we would have >> >> >> #ifndef CONFIG_L2_OFF >>if (get_device_type() != 0xC100) { >>/* turn off L2 cache */ >>l2_cache_disable(); >>/* invalidate L2 cache also */ >>v7_flush_dcache_all(get_device_type()); >>i = 0; >>/* mem barrier to sync up things */ >>asm("mcr p15, 0, %0, c7, c10, 4": :"r"(i)); >> >>l2_cache_enable(); >>} >> #endif >> >> Is this intended? > > maybe not. now we only tested the smdkc100 b
Re: [U-Boot] [PATCH] arm_cortexa8: support cache flush to other soc
Hi, As he goes to home, I reply it instead. On Fri, Sep 4, 2009 at 5:43 PM, Dirk Behme wrote: > Dear Minkyu Kang, > > Minkyu Kang wrote: >> Current code is supported only omap3 soc. >> this patch will support s5pc1xx(s5pc100 and s5pc110) soc also. > > Thanks for this patch! > > How is this patch related to > > http://lists.denx.de/pipermail/u-boot/2009-August/058492.html > It's not good idea to move invalidate_cache to omap directory. we need it. > >> Signed-off-by: Minkyu Kang >> --- >> cpu/arm_cortexa8/cpu.c | 24 +++- >> 1 files changed, 11 insertions(+), 13 deletions(-) >> >> diff --git a/cpu/arm_cortexa8/cpu.c b/cpu/arm_cortexa8/cpu.c >> index 5a5981e..3d430b1 100644 >> --- a/cpu/arm_cortexa8/cpu.c >> +++ b/cpu/arm_cortexa8/cpu.c >> @@ -35,9 +35,6 @@ >> #include >> #include >> #include >> -#ifndef CONFIG_L2_OFF >> -#include >> -#endif >> >> static void cache_flush(void); >> >> @@ -61,17 +58,18 @@ int cleanup_before_linux(void) >> cache_flush(); >> >> #ifndef CONFIG_L2_OFF >> - /* turn off L2 cache */ >> - l2_cache_disable(); >> - /* invalidate L2 cache also */ >> - v7_flush_dcache_all(get_device_type()); >> -#endif >> - i = 0; >> - /* mem barrier to sync up things */ >> - asm("mcr p15, 0, %0, c7, c10, 4": :"r"(i)); >> + if (get_device_type() != 0xC100) { > > Hmm, what is this "0xC100" ? Now we got two cpu, s5pc100 and s5pc110. In case of s5pc100 we don't need to turn off l2 cache. but s5pc110 needs it. So first check the device type, actually cpu type. then determine turn off l2 cache or not. > >> + /* turn off L2 cache */ >> + l2_cache_disable(); >> + /* invalidate L2 cache also */ >> + v7_flush_dcache_all(get_device_type()); >> >> -#ifndef CONFIG_L2_OFF >> - l2_cache_enable(); >> + i = 0; >> + /* mem barrier to sync up things */ >> + asm("mcr p15, 0, %0, c7, c10, 4": :"r"(i)); >> + >> + l2_cache_enable(); >> + } >> #endif > > What's about the order of #ifndef CONFIG_L2_OFF? > > While we had before > > > #ifndef CONFIG_L2_OFF > /* turn off L2 cache */ > l2_cache_disable(); > /* invalidate L2 cache also */ > v7_flush_dcache_all(get_device_type()); > #endif > i = 0; > /* mem barrier to sync up things */ > asm("mcr p15, 0, %0, c7, c10, 4": :"r"(i)); > > #ifndef CONFIG_L2_OFF > l2_cache_enable(); > #endif > > > after this patch we would have > > > #ifndef CONFIG_L2_OFF > if (get_device_type() != 0xC100) { > /* turn off L2 cache */ > l2_cache_disable(); > /* invalidate L2 cache also */ > v7_flush_dcache_all(get_device_type()); > i = 0; > /* mem barrier to sync up things */ > asm("mcr p15, 0, %0, c7, c10, 4": :"r"(i)); > > l2_cache_enable(); > } > #endif > > Is this intended? maybe not. now we only tested the smdkc100 but actual use is internal board for s5pc100 & s5pc110. He will be modify it. Thank you, Kyungmin Park ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm_cortexa8: support cache flush to other soc
Dear Minkyu Kang, Minkyu Kang wrote: > Current code is supported only omap3 soc. > this patch will support s5pc1xx(s5pc100 and s5pc110) soc also. Thanks for this patch! How is this patch related to http://lists.denx.de/pipermail/u-boot/2009-August/058492.html ? > Signed-off-by: Minkyu Kang > --- > cpu/arm_cortexa8/cpu.c | 24 +++- > 1 files changed, 11 insertions(+), 13 deletions(-) > > diff --git a/cpu/arm_cortexa8/cpu.c b/cpu/arm_cortexa8/cpu.c > index 5a5981e..3d430b1 100644 > --- a/cpu/arm_cortexa8/cpu.c > +++ b/cpu/arm_cortexa8/cpu.c > @@ -35,9 +35,6 @@ > #include > #include > #include > -#ifndef CONFIG_L2_OFF > -#include > -#endif > > static void cache_flush(void); > > @@ -61,17 +58,18 @@ int cleanup_before_linux(void) > cache_flush(); > > #ifndef CONFIG_L2_OFF > - /* turn off L2 cache */ > - l2_cache_disable(); > - /* invalidate L2 cache also */ > - v7_flush_dcache_all(get_device_type()); > -#endif > - i = 0; > - /* mem barrier to sync up things */ > - asm("mcr p15, 0, %0, c7, c10, 4": :"r"(i)); > + if (get_device_type() != 0xC100) { Hmm, what is this "0xC100" ? > + /* turn off L2 cache */ > + l2_cache_disable(); > + /* invalidate L2 cache also */ > + v7_flush_dcache_all(get_device_type()); > > -#ifndef CONFIG_L2_OFF > - l2_cache_enable(); > + i = 0; > + /* mem barrier to sync up things */ > + asm("mcr p15, 0, %0, c7, c10, 4": :"r"(i)); > + > + l2_cache_enable(); > + } > #endif What's about the order of #ifndef CONFIG_L2_OFF? While we had before #ifndef CONFIG_L2_OFF /* turn off L2 cache */ l2_cache_disable(); /* invalidate L2 cache also */ v7_flush_dcache_all(get_device_type()); #endif i = 0; /* mem barrier to sync up things */ asm("mcr p15, 0, %0, c7, c10, 4": :"r"(i)); #ifndef CONFIG_L2_OFF l2_cache_enable(); #endif after this patch we would have #ifndef CONFIG_L2_OFF if (get_device_type() != 0xC100) { /* turn off L2 cache */ l2_cache_disable(); /* invalidate L2 cache also */ v7_flush_dcache_all(get_device_type()); i = 0; /* mem barrier to sync up things */ asm("mcr p15, 0, %0, c7, c10, 4": :"r"(i)); l2_cache_enable(); } #endif Is this intended? Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] arm_cortexa8: support cache flush to other soc
Current code is supported only omap3 soc. this patch will support s5pc1xx(s5pc100 and s5pc110) soc also. Signed-off-by: Minkyu Kang --- cpu/arm_cortexa8/cpu.c | 24 +++- 1 files changed, 11 insertions(+), 13 deletions(-) diff --git a/cpu/arm_cortexa8/cpu.c b/cpu/arm_cortexa8/cpu.c index 5a5981e..3d430b1 100644 --- a/cpu/arm_cortexa8/cpu.c +++ b/cpu/arm_cortexa8/cpu.c @@ -35,9 +35,6 @@ #include #include #include -#ifndef CONFIG_L2_OFF -#include -#endif static void cache_flush(void); @@ -61,17 +58,18 @@ int cleanup_before_linux(void) cache_flush(); #ifndef CONFIG_L2_OFF - /* turn off L2 cache */ - l2_cache_disable(); - /* invalidate L2 cache also */ - v7_flush_dcache_all(get_device_type()); -#endif - i = 0; - /* mem barrier to sync up things */ - asm("mcr p15, 0, %0, c7, c10, 4": :"r"(i)); + if (get_device_type() != 0xC100) { + /* turn off L2 cache */ + l2_cache_disable(); + /* invalidate L2 cache also */ + v7_flush_dcache_all(get_device_type()); -#ifndef CONFIG_L2_OFF - l2_cache_enable(); + i = 0; + /* mem barrier to sync up things */ + asm("mcr p15, 0, %0, c7, c10, 4": :"r"(i)); + + l2_cache_enable(); + } #endif return 0; -- 1.5.4.3 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot