Hello Ezequiel,
Thanks, you wrote a full description of what i need to do.. i will
definitely follow this.
On Sun, Aug 5, 2012 at 11:57 PM, Ezequiel Garcia <elezegarcia@xxxxxxxxx> wrote:
> Hi Devendra,
>
> On Sun, Aug 5, 2012 at 1:04 AM, Devendra Naga
> <develkernel412222@xxxxxxxxx> wrote:
>> Hello Ezequiel,
>>
>> On Sun, Aug 5, 2012 at 12:24 AM, Ezequiel Garcia <elezegarcia@xxxxxxxxx> wrote:
>>> Hi Devendra,
>>>
>>> On Sat, Aug 4, 2012 at 3:12 PM, Devendra Naga
>>> <develkernel412222@xxxxxxxxx> wrote:
>>>>
>>>> mutex_init(&ci->lock);
>>>> memcpy(&ci->cfg, cfg, sizeof(struct cxd2099_cfg));
>>>
>>> While you're still looking at this driver, perhaps you can change the memcpy
>>> with a plain struct assignment (if you feel like).
>>> It's really pointless to use a memcpy here.
>>>
>>> Something like this:
>>>
>>> - memcpy(&ci->cfg, cfg, sizeof(struct cxd2099_cfg));
>>> + ci->cfg = *cfg;
>>>
>> Correct, and also one more thing like this is
>>
>> - memcpy(&ci->en, &en_templ, sizeof(en_templ));
>> + ci->en = en_templ;
>>
>> Is it ok if i change ci->cfg and ci->en?
>
> Yes, I believe it is ok.
>
> A few more remarks I would like to add.
>
> 1. When sending patches for staging/media, it's not necessary to put
> staging list/maintainer
> (Greg) on Cc. I guess, it doesn't hurt, though.
> But it's media list / Mauro who will decide on the patches.
>
Yeah, for some patches i have done ccing to Greg, but i think you told me not to
cc at that time itself so not cc'ed now.
> 2. You could also change the order in "struct cxd".
> Currently it's like this
>
> struct cxd {
> struct dvb_ca_en50221 en;
> struct i2c_adapter *i2c;
> struct cxd2099_cfg cfg;
>
> But it would be better to put it like this
>
> struct cxd {
> struct i2c_adapter *i2c;
> struct cxd2099_cfg cfg;
> struct dvb_ca_en50221 en;
>
> It's more logical, and ci->i2c and ci->cfg are used more frequently, so it makes
> sense to put it near the top of the struct.
> (You may think I'm being too paranoid: I am).
>
I am afraid i may not be doing those, if re-ordering may cause some
ambiguous problems. sorry...
> 3. You don't have hw to test, uh?
> In that case, don't forget to always add a "Tested by compilation only"
> inside the commit message. That way the maintainer (Mauro) are free to
> _not_ pick
> the patch, if he feels it's not safe/clear enough.
>
Ok . i will definitely put that message in commit. Thanks
> Hope this helps and thanks for your work,
> Ezequiel.
Since the changes are different than what this patch does, i will do
the changes you proposed in a new patch and will send it out.
Thanks for your time,
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
