Re: [PATCH v3 01/11] MFD: DA9052 MFD core module
On Fri, Aug 05, 2011 at 07:23:44PM +0530, ashishj3 wrote:
Patch v3 seems a little low, we've had *slightly* more versions than
that...
> +choice
> + prompt "Chip Type"
> + depends on MFD_DA9053_SPI || MFD_DA9053_I2C
> +config PMIC_DA9053AA
> + bool "Support Dialog Semiconductor DA9053 AA PMIC"
> + help
> + Support for Dialog semiconductor DA9053 AA PMIC.
> + This driver provides common support for accessing the device,
> + additional drivers must be enabled in order to use the
> + functionality of the device.
> +config PMIC_DA9053Bx
Could do with blank lines between blocks. Though looking at the code
here I don't understand why these are compile options at all, or if they
need to be compile options for some reason why they're not independantly
selectable?
> +int da9052_reg_read(struct da9052 *da9052, unsigned char reg)
> +{
> + int val, ret;
> +
> + if (reg > DA9052_MAX_REG_CNT) {
> + dev_err(da9052->dev, "invalid reg %x\n", reg);
> + return -EINVAL;
> + }
> +
> + #ifdef CONFIG_MFD_DA9052_SPI
> + reg = (reg << 1) | 1;
> + #endif
There's several problems here:
- You shouldn't be indenting preprocessor directives.
- You shouldn't be adding extra indentation before.
- This will break I2C devices if SPI support is built into the driver.
Please, when writing code try to understand the abstractions you're
using. For example here think about the purpose of being able to build
I2C and SPI separately and simultaneously and the goal of the regmap
API.
Looks like we need to add per device mangling for the SPI register
read/write flag.
> +static struct mfd_cell __initdata da9052_subdev_info[] = {
> + {"da9052-onkey", .resources = &da9052_onkey_resource,
> + .num_resources = 1},
> + {"da9052-rtc", .resources = &da9052_rtc_resource,
> + .num_resources = 1},
> + {"da9052-gpio"},
> + {"da9052-hwmon"},
> + {"da9052-leds"},
> + {"da9052-WLED1"},
> + {"da9052-WLED2"},
> + {"da9052-WLED3"},
Upper case in device names is *very* non-idiomatic.
> + {"da9052-tsi", .resources = da9052_tsi_resources,
> + .num_resources = ARRAY_SIZE(da9052_tsi_resources)},
> + {"da9052-bat", .resources = da9052_power_resources,
> + .num_resources = ARRAY_SIZE(da9052_power_resources)},
> + {"da9052-watchdog"},
> +};
The formatting here is really odd for Linux, both in terms of the lack
of spaces and the brace placement.
> + da9052_group_write(da9052, DA9052_EVENT_A_REG, 4, v);
> +
> + #ifndef CONFIG_PMIC_DA9053Bx
> + DA9052_FIXME();
> + #endif
This should be runtime detected based on the device name, either from
the device registration or by reading back chip identification.
Formatting issues too.
> + return 0;
> +}
> +module_init(da9052_spi_init);
I rather suspect that you'll find that in order to use this in an actual
system this needs to be subsys_initcall().
> +MODULE_ALIAS("platform:da9052_spi");
This isn't a platform driver.
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: Switching to Gerrit for Android code hosting - last stage
On Fri, 5 Aug 2011 15:57:59 +0100, Paul Sokolovsky wrote: > Hello, > > Following today's work session preparing to switching Gerrit, everything > is ready for the cutover now: > > First build from Gerrit repositories is finished successfully: > https://android-build.linaro.org/jenkins/job/pfalcon_beagle-gerrit/4 > > Gerrit Web UI is migrated to the final location: > http://review.android.git.linaro.org/ > > Gitweb is installed at > http://android.git.linaro.org/gitweb > > Anon git access to repositories in Gerrit available via > git://android.git.linaro.org/... > > > It's too late to do the cutover now of course, so I'd like to propose > to do it on Wed 10th to be sure that all folks are back from the > Connect and final bits are settled. Hi, This sounds fine to me. Do you anticipate that this step will require any coordination with IS? Thanks, James ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: thumb compiler flags
Bero, I think this might be a good patch to get up to AOSP after we try it out. Assigning a bug for Chao to look at: https://bugs.launchpad.net/linaro-android/+bug/822113 -Zach On 2 August 2011 22:09, Bernhard Rosenkranzer wrote: > Hi, > I just spotted this bit in Android's Makefiles (inherited that way from AOSP): > > TARGET_arm_CFLAGS := [...] -fstrict-aliasing [...] > [...] > TARGET_thumb_CFLAGS := [...] -fno-strict-aliasing [...] > > This general assumption that arm code can handle strict aliasing, but > thumb code can't, seems odd. > > My guess is that they started out using -fno-strict-aliasing > everywhere, then fixed aliasing violations and "fixed" the compiler > flags for arm, and didn't notice they're still turning it off for > thumb -- either that, or they've run into toolchain bugs (that might > be fixed in the Linaro toolchain), or it's a matter of "something > we're building in thumb mode has aliasing violations and we're too > lazy to figure out which part it is". > > Unless someone knows of a valid reason for doing things the way they > did, simply taking it out (preferrably replacing it with > -Werror=strict-aliasing, so we're likely to notice any aliasing > violations if it comes down to "too lazy to figure out which part") > should get us some free extra speed. (Not sure just how much we're > actually building in thumb mode - may or may not be relevant.) > > ttyl > bero > > ___ > linaro-dev mailing list > linaro-dev@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/linaro-dev > ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: Problems with linaro-android_toolchain-4.6-2011.07 rebuilds
I'm seeing the same thing: $git push linaro HEAD Counting objects: 5, done. Delta compression using up to 4 threads. Compressing objects: 100% (2/2), done. Writing objects: 100% (3/3), 454 bytes, done. Total 3 (delta 1), reused 0 (delta 0) error: insufficient permission for adding an object to repository database ./objects fatal: failed to write object error: unpack failed: unpack-objects abnormal exit To ssh://pfeff...@git.linaro.org/srv/git.linaro.org/git/android/toolchain/manifest.git ! [remote rejected] HEAD -> toolchain-11.07-release (n/a (unpacker error)) error: failed to push some refs to 'ssh://pfeff...@git.linaro.org/srv/git.linaro.org/git/android/toolchain/manifest.git' Any ideas Paul? On 7 August 2011 01:20, Bernhard Rosenkranzer wrote: > Hi, > I have a manifest that reverts everything to what it was in 11.07 > ready to go - but I seem to lack permissions to push it to a new > branch: > > $ git push linaro toolchain-11.07-release > Counting objects: 5, done. > Delta compression using up to 8 threads. > Compressing objects: 100% (2/2), done. > Writing objects: 100% (3/3), 477 bytes, done. > Total 3 (delta 1), reused 0 (delta 0) > error: insufficient permission for adding an object to repository > database ./objects > > fatal: failed to write object > error: unpack failed: unpack-objects abnormal exit > To > ssh://bernhardrosenkran...@git.linaro.org/srv/git.linaro.org/git/android/toolchain/manifest.git > ! [remote rejected] toolchain-11.07-release -> > toolchain-11.07-release (n/a (unpacker error)) > error: failed to push some refs to > 'ssh://bernhardrosenkran...@git.linaro.org/srv/git.linaro.org/git/android/toolchain/manifest.git' > > Can someone fix this or should I just use master and revert later? > > I've attached the relevant file if anyone wants to push it (it gets > back to 11.07 by simply specifying the correct revisions to pull -- no > need to revert the other repositories). > > ttyl > bero > > On 7 August 2011 01:40, Zach Pfeffer wrote: >> I filed a bug on this and assigned it to bero: >> >> https://bugs.launchpad.net/linaro-android/+bug/822106 >> >> Bero once you're happy with the original build I can cut an 11.07. It >> would be nice t have git and a pinned-manifest to go back to. >> >> -Zach >> >> On 5 August 2011 18:04, Paul Sokolovsky wrote: >>> On Fri, 5 Aug 2011 17:12:50 +0200 >>> Alexander Sack wrote: >>> On Fri, Aug 5, 2011 at 4:13 PM, Paul Sokolovsky wrote: > On Fri, 5 Aug 2011 15:54:22 +0200 > Alexander Sack wrote: > >> ok. seems it really happened. >> >> https://android-build.linaro.org/jenkins/job/linaro-android_toolchain-4.6-2011.07/8/artifact/build/out/android-toolchain-eabi-linaro-4.6-2011.07-0-8-2011-07-25_12-42-06-linux-x86.tar.bz2 >> >> doesnt exist anymore even though its used as our official download >> url for release. >> >> Please figure out what changes landed after that official build, >> revert everything to the release state, respin, and ask paul to put >> the original binary manually back in place so the above URL becomes >> valid again? > > Well, it's built from the release tarball and the same patch, both > stay the same, so end result of the rebuild is the same, just at > different URL (build #12). I've updated daily jobs to use that, and > personal jobs should be updated by developers as needed. I dont think that build is the same. bero added changes somewhere etc. otherwise i doubt he would have respun the built. Please check with him. >>> >>> I see, that's indeed the case. So we apparently need to branch/tag >>> release builds for toolchain just the same as for the platform. In the >>> meantime, we with Bernhard are working on resolving this and bringing >>> original 11.07 build back up. >>> >>> >>> >>> -- >>> Best Regards, >>> Paul >>> >>> Linaro.org | Open source software for ARM SoCs >>> Follow Linaro: http://www.facebook.com/pages/Linaro >>> http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog >>> >> > ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
