On Mon, Mar 2, 2015 at 5:54 PM, Vince Hsu <[email protected]> wrote:
>>> +int tegra_mc_flush(struct tegra_mc_swgroup *sg)
>>> +{
>>> + struct tegra_mc *mc;
>>> + const struct tegra_mc_hotreset *client;
>>> + int i;
>>> +
>>> + if (!sg || !sg->mc)
>>> + return -EINVAL;;
>>> +
>>> + mc = sg->mc;
>>> + if (!mc->soc->ops || !mc->soc->ops->flush)
>>> + return -EINVAL;;
>>> +
>>> + client = mc->soc->hotresets;
>>> +
>>> + for (i = 0; i < mc->soc->num_hotresets; i++, client++) {
>>> + if (sg->id == client->swgroup)
>>> + return mc->soc->ops->flush(mc, client);
>>> + }
>>> +
>>> + return -EINVAL;
>>> +}
>>> +EXPORT_SYMBOL(tegra_mc_flush);
>>> +
>>> +int tegra_mc_flush_done(struct tegra_mc_swgroup *sg)
>>> +{
>>> + struct tegra_mc *mc;
>>> + const struct tegra_mc_hotreset *client;
>>> + int i;
>>> +
>>> + if (!sg || !sg->mc)
>>> + return -EINVAL;;
>>> +
>>> + mc = sg->mc;
>>> + if (!mc->soc->ops || !mc->soc->ops->flush)
>>> + return -EINVAL;;
>>> +
>>> + client = mc->soc->hotresets;
>>> +
>>> + for (i = 0; i < mc->soc->num_hotresets; i++, client++) {
>>> + if (sg->id == client->swgroup)
>>> + return mc->soc->ops->flush_done(mc, client);
>>> + }
>>> +
>>> + return -EINVAL;
>>> +}
>>> +EXPORT_SYMBOL(tegra_mc_flush_done);
>>
>> These functions are identical, excepted for the callback they are
>> invoking. Could you merge the common part into a function that returns
>> the right client to call the callback on, or ERR_PTR(-EINVAL) in case
>> of failure?
>
> I couldn't think of a clever way to do this. Any ideas? :)
How about something like this (warning: might now be that great, untested):
/* Have this in your .h and use it in your tegra_mc_ops struct */
typedef int (*mc_op)(struct tegra_mc *mc,
const struct tegra_mc_hotreset *hotreset)
static int __tegra_mc_op(struct tegra_mc_swgroup *sg, mc_op op)
{
struct tegra_mc *mc;
const struct tegra_mc_hotreset *client;
int i;
mc = sg->mc;
client = mc->soc->hotresets;
for (i = 0; i < mc->soc->num_hotresets; i++, client++) {
if (sg->id == client->swgroup)
return op(mc, client);
}
return -EINVAL;
}
#define tegra_mc_op(sg, op) \
((!sg || !sg->mc || !sg->mc->soc->ops || sg->mc->soc->ops->op) ? \
-EINVAL : \
__tegra_mc_op(sg, sg->mc->soc->ops->op));
int tegra_mc_flush(struct tegra_mc_swgroup *sg)
{
return tegra_mc_op(sg, flush);
}
EXPORT_SYMBOL(tegra_mc_flush);
int tegra_mc_flush_done(struct tegra_mc_swgroup *sg)
{
return tegra_mc_op(sg, flush_done);
}
EXPORT_SYMBOL(tegra_mc_flush_done);
Actually when writing this code I found two other issues:
>>> +int tegra_mc_flush_done(struct tegra_mc_swgroup *sg)
>>> +{
>>> + struct tegra_mc *mc;
>>> + const struct tegra_mc_hotreset *client;
>>> + int i;
>>> +
>>> + if (!sg || !sg->mc)
>>> + return -EINVAL;;
Double ; (also in tegra_mc_flush)
>>> +
>>> + mc = sg->mc;
>>> + if (!mc->soc->ops || !mc->soc->ops->flush)
Should be ops->flush_done?
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
